ChessTDD 20: Refactoring in Earnest

In this post, I set out to do some real refactoring of the Board class.  It bothered me enough to take a crack at it and, since this is a fun side project, there really aren’t any constraints.  Were I committed to delivering some business value here, I might need to take a look at my priorities and evaluate whether making things clearer here is worth the delay.  But, luckily in this case, I don’t need to make that call.  And, refactorings are always fun.

Here is what I accomplished in this clip:

• Refactored a couple of methods out of Board and onto BoardCoordinate.
• Refactored path checking logic into a PathChecker class.

Here are lessons to take away:

• If you have functionality that’s purely static in some class (as in, not referring to instance variables in that class), think about where else it might go.  If that static method is principally interested in operating on properties or with methods of another type, you might have the “feature envy” code smell.  This was the case with my static methods that evaluated whether two BoardCoordinates were on the same horizontal or vertical path.  They compared properties on two different BoardCoordinates — so why not make this a member of BoardCoordinate?
• This isn’t really something I’ve been emphasizing here, but early on I decided to do a quick local commit with Git.  Commit early and often.  I’ve never regretted too many commits unless I was using a terrible source control tool.
• Defining  a class in the same space as another class is a tool that can help me with extract class refactorings.  This is one of the more volatile refactorings that you can do, so make sure you don’t try to do too much at one and that you get green regularly as you go.  Recreate the functionality faithfully first and then think about how to refactor.
• If you can factor a method toward not referring to any instance state, then you’re well on the way to letting it be moved to a different class.  This is a good intermediate step to reason about — see how much instance state you can abstract out.
• When you extract a class as part of a refactoring, it’s fine to leave the tests that cover both classes in place.  In fact, it’s a natural set of tests to leave.  Add tests for the newly created class only to address new complexities that arise from the newly exposed functionality needing to tighten up guard conditions and such.