tulir / fifteen Goto Github PK

Repository cloned 2019-02-15 13:55, commit ...281ec5

I would like to preface my comments by noting that I've never coded in Go, so I
may have misunderstood some of the code. Caveat emptor!

As given the build instructions do not produce an executable on my machine,
since the build command tries to produce an executable named 'fifteen', but
this name conflicts with the source directory name. I was able to circumvent
this by creating a build directory and/or by using the -o switch to rename
the target executable.

Overall, the code is nicely structured. Some functions could use a little more
documentation to make what's being done more apparent (e.g. Solvable(), I only
have a vague idea of what is being done there after looking into 15 puzzle
solvability), but all the functions have a brief descriptions, good job!

The animated UI is a cool idea, even if it isn't central in terms of course
content. However, during my testing I was unable to get it to show the solver
taking steps (it only seems to show the first and last state). The UI code is
predictably complicated, and it's the part I spent the least time reviewing, so
it could be that I was just using the wrong flags or something.

The division of labor between the Position data structure and Puzzle is a bit
counter-intuitive in my opinion. In particular, I think the move validity is
probably something I would do in the Puzzle class, since in it will also depend
on the position of the blank (see also bug 2 below).

I found a couple bugs in the code:

1. In the NewSolvedPuzzle function, the position of the blank is always set to
   (X,Y)=(4,4), which may be outside the data array (3x3 puzzles) or occupied by a
   non-blank, when the puzzle size is greater than 4x4. This will also lead to
   strange results with Move(), because the function will move the false blank
   around. As I understand it, the following test should not fail (but does!):

func TestPuzzle_BlankValue(t *testing.T) {
	for i := 3; i<16; i++ {
		puzzle, _ := NewSolvedPuzzle(i)
		assert.Equal(t, 0, puzzle.Get(puzzle.blank.X, puzzle.blank.Y));
	}
}

2. This is not necessarily a problem, since the Move() method is only called by
   your own code and not "user-facing", but I think it might be worth checking if
   a move is "game valid" (i.e. next to the blank) in the Move() method. Checking
   for this could catch hidden bugs in other parts of the code.

3. The Parser class could use a little more checking. The parser does not check
   for repeated numbers or blanks, or for the existence of a blank. Also, the blank
   position is not set. In particular the following tests should not fail (there
   should also be a test for checking that the blank position is set correctly):

func TestParsePuzzle_RepeatedValue(t *testing.T) {
	puzzle, err := ParsePuzzle(
		"1 1 1\n" +
		"1 1 1\n" +
		"1 1 1")
	assert.Error(t, err)
	assert.Nil(t, puzzle)
}

func TestParsePuzzle_MultipleBlanks(t *testing.T) {
	puzzle, err := ParsePuzzle(
		"- 2 3\n" +
		"4 5 6\n" +
		"7 8 -")
	assert.Error(t, err)
	assert.Nil(t, puzzle)
}

Time when first cloned project was 21.2 and no changes to code has been made when this report has been written.

Getting the project to work on my windows computer was a bit of a roller coaster ride. It started off smoothly with the installation working properly.

Go installer automatically added PATH to windows so I was really confused when the following problems emerged.

It turns out it was my mistake with syntax in the screenshot above, however even when I did fix the problems powershell/command line would not work properly with this project.

Thankfully I got direct help from Tulir and we eventually sorted things out. He suggested that I use a more linux like terminal. I tried git bash and it worked out.

I unknowingly discovered a bug with my exploratory testing. Tulir mentioned that the panic: runtime error is a json file input reading error.

After all the struggle I finally got the program to work with plain text solution.

This is the first time ever that I read the Go programming language. At a quick glance nothing else say about the quality of the code, except it looks pretty good. I have no idea how code structure works in Go, but all the files and test files are in the same giant folder which seems a bit messy.

P.S Played some version of this game when I was small. Would have been great to play a little bit to try to solve some of the puzzles, then coming back and using this program. Unfortunately ran out of time.