I start to peruse the source. Python programs are written a mix of paradigms, and this is something I love about it. Every variable is a first-class object, but not all methods have to be members of a class. You can have global functions (well, module-scoped) which are themselves first-class objects capable of having multiple methods and member variables, all participating in reflection/introspection. As such, some will describe the use of non-class-bound methods as “procedural”, and I suppose it’s technically a fair cop. Some of the program is written in class-free functions, mostly the bits that are used to initialize the application.

Python also has a number of functional programming features, but I suspect that the 2002 copyright predates list comprehensions and generators, so I don’t see them represented here. I will be using some of those techniques to simplify code that had to be written in loops and nested loops before. It should shorten a number of methods.

The PyGame UI and business logic (guts) of the code are separate modules with a protocol for communicating between the two. I was afraid that I would find them all intermingled, and was delighted to see separate modules. The core of the game itself is only a few hundred lines of code if I include comments, docstrings, and whitespace.

I know that the first order of business is to get some testing started, but I want so badly to to take Eclipse PyDev for a spin that I’m having an impulse control problem. I think the code is formatted reasonably, but I use the Eclipse feature to reformat the code to see if it works. I’m rewarded with equally attractive code. No damage is done!

The outlining feature works fine, and so does completion. I scribble a little and delete it. It’s looking good so far. I notice that the Run As.. menu has options to run Python, Jython, PyUnit, Python coverage and more. It’s looking good.

What of refactoring? Sure enough I find that I can rename, extract and inline variables, and extract and inline functions. Those are the biggies. I scroll into the constructor of the Field class. The Field represents the playing field of the game, the grid of bombs and clues. In the constructor are various paragraphs of code that help set up the class. I pick the section that adds mines, move some of the constants out of the way by extracting local variables, and then extract that portion of the function as _place_mines. In Python no functions are private, so the standard protocol is to use an underscore prefix as a ‘wart’ to tell other authors that it’s not really part of the formal interface, just a convenience function for the class. Love it or hate it, that is just how it’s done.

I do the same with the paragraph that validates the parameters (ie there are more than zero rows, columns, and mines, and that there are not more mines than cells in the grid). This becomes _validate_parameters, and is called from the constructor (in Pythonese the constructor is a function named __init__).

I find eight instance variables in __init__. That’s quite a few. I pledge to look for violations of the Single Responsibility Principle later.

It dawns on me that Eclipse doesn’t know my current version control selection, <a href= “http://en.wikipedia.org/wiki/Bazaar_(vcs)”>bazaar. This distresses me, because all the Team features of Eclipse are lost to me. I won’t be able to do all my checkin/update/diff functions from within the IDE. I vow to look for a plugin soonest. That’s two promises, two items of debt so far. I will keep going today.

For now, I see a function called _get_adjacent. It has a pretty simple function, but it looks like this:

    def _get_adjacent(self, x, y):
        """Provide a list of all tiles adjacent to the given tile.

        This function takes the x and y coordinates of a tile, and returns a
        list of a 2-tuples containing the coordinates of all adjacent tiles.

        x and y are the x and y coordinates of the base tile, respectively.
        """ 
        adjlist = [(x - 1, y - 1), (x, y - 1), (x + 1, y - 1),
                   (x - 1, y), (x + 1, y),
                   (x - 1, y + 1), (x, y + 1), (x + 1, y + 1)]
        rmlist = []
        for adjpair in adjlist:
            for value, index in [(-1, 0), (-1, 1), (self.cols, 0),
                                 (self.rows, 1)]:
                if adjpair[index] == value:
                    rmlist.append(adjpair)
                    break
        for item in rmlist:
            adjlist.remove(item)
        return adjlist

It looked pretty heavy to me, for the small amount of work it’s doing. Ah, it had to use nested loops instead of list comprehensions, and it’s adding unnecessary rows only to remove them later. I think I could do better if I had a body of tests to protect me.

I figure it’s time to try PyUnit inside Eclipse. I create a new file called testPysweeper.py. I hate the name, and again go against my better instincts by leaving it that for a while. That would be yet another promise, and I’m running out of patience for procrastination. I rename it testAdjacency.py. I feel an instant sense of relief and go on to write a series of tests of the existing _get_adjacent method. It is

        class PySweeperTest(unittest.TestCase):
            def parentSetup(self, columns, rows, mines, target):
                target_column, target_row = target
                self.game = pysweeper.Field(columns,rows,mines)
                self.target = (target_column,target_row)
                self.result = self.game._get_adjacent_cells(*self.target)
                self.result.sort()

        class CellsAdjacentToCol2Row2(PySweeperTest):
            """Expecting a circle of squares around the target cell""" 
            def setUp(self):
                self.parentSetup(5,5,0, (2,2))
            def testShouldFindEightCells(self):
                assert len(self.result) == 8
            def testShouldNotIncludeCol2Row2(self):
                assert self.target not in self.result

These are not highly imaginative, and continue with several different scenarios. I test a normal “middle” cell, one against an edge, the top left and bottom right corners, etc.

You’ll notice the base class PySweeperTest. I didn’t have that to start with but added it to reduce duplication, and I named it badly. These are not general PySweeper tests, but they are specific cell adjacency tests. Naming is easily fixed.

A cool thing happens. I rename it AdjacencyTest and Suddenly all the test names become redundant and silly. I start renaming them, and

  class CellsAdjacentToCol0Row0(PysweeperTest):
  

becomes

  class Column0Row0(AdjacencyTest):
  

On further reflection, this becomes:

  class UpperLeftCorner(AdjacencyTest):
  

It’s amazing how wrong my first instinct can be. The test is so much more obvious now, and the conditions make so much more sense. I smile in mild self-derision, but I’m happy to have cleaned it up quickly. The PyDev refactoring tools feel pretty comfortable to me now. One goal of the exercise is being met.

The tests run for the old routine, and I write a new routine.

I hit my next snag trying to run the python unit test. I have always done python from the command line, and would run my tests as if they were any other python program. I never did them as:

        python -m unittest filename.testname

I think that this must be what Eclipse wants to do. I can use the option to Run as.../Python run but I cannot use the option to Run as.../Python unit-test. When I try I get the following error:

  Finding files... ['/home/tottinge/Projects/Pysweeper-1.0/testAdjacency.py']
  done.
  Importing test modules... done.

  ----------------------------------------------------------------------
  Ran 0 tests in 0.000s

It found the right test file, but either it didn’t import it, or else it imported it and somehow didn’t recognize any tests. I added a print line to the file so I could see it, and got nothing. Why would it find my file and not import it? This is something that requires a little google..

It doesn’t work when I use the python -m unittest blah either. Maybe the problem isn’t that I’m not used to PyDev, but that I don’t know enough about PyUnit? I put this off, since I have a reasonable workaround for now. This is another bit of debt to work off. I’m unhappy at how fast it piles up.

The old version of _get_adjacent was about 21 lines with loops and local variables. Here is a smaller version using list comprehensions:

   def _get_adjacent(self, x, y):
        """Provide a list of all tiles adjacent to the given tile.""" 
        columns = (x-1,x,x+1);
        rows = (y-1,y,y+1);
        adjacent_pairs = [ (col,row)  for col in columns 
                                      for row in rows 
                                      if 0<=row<self.rows and 0<=col<self.cols]
        adjacent_pairs.remove( (x,y) )
        return adjacent_pairs

A list comp is very simple. The one above returns a list of (row,column) pairs by iterating over columns and rows, and only returns a pair if the row and column are both on the grid. But it doesn’t look that simple above. My spider sense buzzes. I immediately worry that the list comp is too complex. The filtering is too deep in the list comprehension and is too far from the initialization of rows and columns. A quick modification and retest.

    def _get_adjacent_cells( self, x, y ):
        """Provide a list of all tiles adjacent to the given tile.""" 
        columns = [ col for col in ( x-1, x, x+1) if 0<=col<self.cols ]
        rows = [ row for row in (y-1, y, y+1) if 0<=row<self.rows ]

        adjacent_pairs = [ ( col, row )  for col in columns for row in rows ]
        adjacent_pairs.remove( ( x, y ) )

        return adjacent_pairs  

The tests all pass, and the code is more obvious to me. I have no teammate to correct me, so I leave it as-is.

One more feature to look at today, and that’s the compare with local history. It’s a beautiful thing. Eclipse is becoming a very good friend to me.