Refactoring Complicated Code
From Brute Force to Formal Elegance
Here’s the situation: you’ve been hammering away at a complicated coding problem for a day or so and you’ve iterated your way into a working system. Mission complete, right?
Slow down, friend. It’s time to clean up that code before you go much further.
Finding a brute force solution is not the end of the line. It’s worthwhile to take a moment and look for ways to streamline what you’ve created. Search for repeated code, and possibly isolate that code into its own method. Look at where information starts and stops, and see if you can move things around to eliminate dependencies. Explore different data structures to see if there is one that can help you avoid costly operations.
Today, I’m going to do some cleanup of the Tactical Command System I outlined in the previous two articles. Most crucially, I’m going to refactor the command execution system to require less information passed up to the manager by using the Command Pattern.
Out With The Old
The first thing we need to examine is the end of the line. We have two methods involved in executing actions. The ProcessRoundRoutine collects all the relevant information and feeds it to a catch-all execute method.
As you can see, this is too much responsibility for one class. We don’t need to pass all the information to one location to execute it. We can let the objects that already hold this information execute their own actions. We just need to get a list of those actions encapsulated into a single object, and then pass that object. That’s the essence of the command pattern. So let’s prepare an interface and a series of command objects.
In With The New
Here we define the contract for this interface to include adding and removing queued actions, as well as doing and undoing them. We also add a property for ExecutionTime which will become more relevant once animations are added. For now, it serves to keep actions from playing over the top of one another.
Now, each type of action still needs its own version of the execute method so we’ll create a command for each.
Here’s the MoveObjectCommand script as an example:
And of course we need a method to add the command to a list:
At this point we can see that there is no longer a purpose for the Turn class. Here’s what it used to contain:
All of this information is available to the actor, who is now responsible for constructing and executing their own actions. We can ditch this class entirely, and simply use the _commandBuffer as a list of pending actions.
Finally, this brings us to our newly refurbished ProcessRoundRoutine coroutine:
Now that’s much more streamlined. Most of the action happens in one line here: currentActionNode.Value.Execute(); That’s fantastic. No more ambiguously generic object types to check, fewer lists of lists to run through, fewer dependencies in general.
Using this general process, be sure to take time to clean up and refactor your code on occasion. Make sure you’re working on a safe branch of your repository — it’s easy to break something while you’re “fixing” it. It can be easy to work out a brute force solution and leave it there, but you might just be making things harder for yourself in the long run if you do. It’s better to refactor a new system you just built than to be forced into it later.
That’s all for today. In the next article we’ll work out how the imperiled AI victims will decide what to do with themselves while they wait to be saved.
