Nim Refactor Wiki

In the Nim game, players remove either one, two, or three sticks at a time from a pile. The player to remove the last stick wins. If you know the secret, you can win every time. See http://en.wikipedia.org/wiki/Nim for more info.

Note from Mr Becker: This program was originally created by Mian Uddin. He presented it at a recent CSC meeting. I have forked the code from his Git repository to the LoganCSC organization in order to make a series of refactoring changes. I have documented the changes I made below and provide a rationale for why I made the changes I did. These changes will not be reflected in Mian's original code, unless he decides to pull them.

Here is a summary of the changes:

• Removed compiled .class and .jar files. Usually these are not checked in with source code, as they take a lot of space and can always be rebuilt from the source.
• Moved nim_game.java to a src folder. This convention conforms to intellij, maven, gradle, and other java build tools. It also helps to organize the project directory by keeping all source files in a single folder.
• Added a .gitignore file to control which files are actually managed by Git.
• Renamed nim_game to NimGame to adhere to standard java naming conventions. Following standard coding conventions is more important than you might think.
• Fixed bug where if (stickCount = 0) should have been if (stickCount == 0). Verified that the program compiles and runs correctly.
• Made a number of small corrections and simplifications pointed out by Intellij.
• Used spaces for indentation instead of a mix of tabs and spaces. It does not matter much whether you use tabs or spaces, but it is very important to be consistent.
• Discovered a bug: If you enter 3 for each player until there is two left, and then enter 3 again (even though there are only 2 sticks), the game will not terminate. It keeps asking for a number of stick to pick up even though there are none left. I will wait to add some unit tests before I fix this.
• Made the NimGame class more object oriented. Instead of having all static methods, there is now an implicit constructor and a "startPlaying" method. The main class creates an instance of the NimGame and starts it playing. The advantage of this approach is that now the NimGame can be used elsewhere (outside this class), and you could even have multiple NimGames running simultaneously. Avoid static methods - they are not OO.
• This is probably the most important refactor. If you don't look at anything else, please read this. Pulled the game state out into a separate NimGameState class. This is important because it not only makes the code much simpler to read and understand, but now the NimGameState can be independently unit tested. Look closely at the NimGameState class. It encapsulates all the state information about the game and contains no print statements. All the data is private, and only a minimal set of methods are exposed. Instead of exposing the properties publicly, or using setters and getters, the methods correspond to natural game actions, and may update multiple properties at once. For example pickUpSticks(num) adjusts the number of sticks remaining, advances to the next turn, and returns true if the current player just won. Also fixed the bug where the game does not terminate if the last move picks up more sticks than remain. Note that the NimGameState and its methods are "package private" (not public) so only other classes in this package can access them. There is an expectation that no class besides NimGame would need to access NimGameState.
• Pulled out all the code for playing the Nim game into a separate method. This makes it easier to read by clearly labeling all the code that relates to playing a single game. The original long method now looks like this:

    public void startPlaying() {
        System.out.println(JAVA NIM);
        while (playGame()) {};
    }

• Test on "won" instead of "!won" in conditional to make code easier to read. Switched the if/else blocks to match.
• Created a bunch of unit tests to exercise the NimGameState code and be confident that it works before making additional changes. In the process of adding tests, I found and fixed a bug where the number of sticks could drop below 0. This change introduces a dependency on junit. An alternative would have been to add a main method to NimGameState with unit tests in it. However junit is a convenient, lightweight library, and its better to keep unit tests out of the production code. All unit tests should be run before every commit. If you use gradle, the tests are run every time you compile.
• Added a constant for the max number of sticks that can be picked up.
• Added ability to check the game state and see if the game is over. Added tests for it. This allows refactoring out a doPlayerTurn method in NimGame that returns true if play is to continue.
• Made Scanner global to the NimGame class to it does not have to be passed around to every method. There may be differences of opinion on whether or not this is really a good refactor, but I think it is justified.
• Refactored out several small nicely named, single purpose methods in NimGame to enhance readability and maintainability.
• Renamed some variable names and methods to be more descriptive. Descriptive names mean you need fewer comments.
  • playing -> continuePlaying
  • input -> numSticks
  • switchToNextPlayer -> advanceToNextPlayer (works better if multiple players added)

Refactoring changes are incremental. Don't try to change to much at once. Make one small improvement, test, and then commit it. That way, if something goes horribly wrong, you can always back up to your last change, and not lose work.

I hope this gives you an idea of the process that you should go through as you create programs. The program runs exactly the same as it did before any changes were made (minus a few bugs), but the code should be easier to understand and maintain now. Given where the code is now, after these changes, what ideas do you have to further improve the readability? What new features could we add? Is it easier to maintain now? Why?

Note that Intellij, Eclipse, and other IDE's have powerful refactoring capabilities built in. These features allow you do to things like right click and rename a variable or method such that every single instance of that variable or method is renamed, at once, in all the files that is used in throughout the codebase. Another great one is to select a block of code and do right click extract method. It will then automatically rewrite the code with that block as a separate method with appropriate parameters.

Here are some ideas for new features:

• Have the user specify the number of starting sticks at startup.
• Make a graphical interface for it. Note that NimGameState will not change. In fact, you can rename NimGame to TextNimGame and create a new GraphicalNimGame - both of which use the same NimGameState class. This is a great example of code reuse. Never copy code logic.
• Allow multiple players to play.