Refactoring my “Fire in the Lake” software

Although I had been refactoring bunches of code present in some methods into other methods, and created a couple additional structs when the original structs were handling concerns beyond what they were originally created for, at this point of the project some structs were accumulating massive amounts of methods. Refactoring them would require identifying the general purposes of the methods involved, classifying them and then isolating them from the original code so that area of the codebase wouldn’t be so brittle. I focused first in the area that handles executing the player or bot commands. As a general overview, each of the four players will push through the mouth of the system some words that will relate to their intentions, what they want to do in this turn. If they push through words like “event”, “operation”, “pass”, the system needs to understand that the player wants to play the active card for the event, or wants to perform an operation, or intends to pass. However, we also have words like “6”, “an loc”, “saigon”, “pacify”, “rally”. The system should reliably handle what kind of operation the player wants to play, in what space or spaces of the board, or stuff like how many troops it wants to deploy. In my initial design I simply passed an array of Strings through the command execution system, and those methods had the responsibility to figure out the intention of the player from whatever subset of words the method received. That was a clear violation of the separation of concerns, given that those methods should simply have to focus on executing an operation, a special activity, or delegating declaring that player as having passed, for example.

Before dealing with that major issue, I needed to take some scissors to the execute commands function. The mess of switches and ifs ended up being distributed into isolated functions that handled executing the events, others that executed each kind of operation, others that executed each kind of special activity, another that handled passing, and the biggest group, one that handled atomic executions such as deploying some units somewhere, manipulating the resources of a faction, setting a space to a level of support, improving the trail that the NVA faction uses, etc. That refactorization into a couple dozen files passed the integration tests without much trouble; thankfully, the three almost end-to-end tests that I wrote and that simulated three entire turns of the game are very resistant to refactorization.

There was another section of the code that handled the entirety of the flow of the game: how to deal with the couple of current cards, how to determine what factions were eligible or ineligible, how to slot those factions so that other parts of the system would know that the choices of those factions were already occupied, etc. It ended up being a tremendous chunk of code. I found three major different concerns for the methods involved: some methods just handled the general game flow: setting the active card, informing whether the turn had ended, moving to the next eligible faction according to the order shown in the active card, taking in the general player choices, being able to answer to whoever asked whether the turn had ended or not, etc. Another concern involved handling all the different possibilities for the factions: whether some faction is eligible, whether all are eligible, whether some faction has passed, determining the eligibility of a faction depending on some previous one’s actions, etc. The final concern was related with slotting the factions and moving them around into “holes” that can only hold either one of them at a time or four (maximum number of players). Whether a player has chosen, for example, to perform an operation without a special activity, is very important in this game system, because that means that the following player can only do a limited operation, while if the first player had chosen to play the card’s event, the second player would have been able to do a full operation plus a special activity.

The most pressing concern regarding the following features I had to implement had to do with the fact that I was passing an array of words from the “mouth” of the system to the lowest levels that handled command execution (in some cases to the very lowest level of executing atomic commands). It reeked of primitive envy; what the system should truly know is whether the player intended to perform one of the main choices (passing, event, operation), what operation if the player went that route (ex. rally, train, sweep), whether the player chose to do the additional action allowed for some of the main operations (such as improving the trail), or if the player chose a special activity, which one. We also have the fact that the player might have written the names of spaces (can be cities, provinces or lines of communication), and the program has to figure out if those locations are related to the event, the operation or the special activity. So I refactored out the entirety of that system and created an interpreter that chews the initial commands and just registers the player intentions. It can be asked “does the player want to activate the event?”, which returns a boolean response, and it can be asked to provide the locations associated with the event, for example. Far, far cleaner than the original, system, and not particularly hard to write. It suffers from a case of “excess of switches/ifs”, but it’s not pressing to figure out how to refactor that out. It reads each word as it comes, and depending on what intentions had been “detected”, it will convert to internal enums stuff like the names of places. If it receives the name of a space, the program determines whether, for example, the name actually refers to a space, and in that case if the player had previously sent the word to perform a operation, and in that case if it also had sent the word to perform a special activity. In that case the following spaces would be sent to a list of spaces associated with the special activity, but they would get associated to the operation or the event in other cases.

Again, thanks to the integration tests, the first time I got the major changes to compile, they immediately passed all the tests, so I knew that the previously codified functionality remained intact. That’s the advantage of test driven design: you can dismantle main areas of the code, improve it substantially, and remain confident that everything keeps working as it should. You wouldn’t dare to risk major changes otherwise.