Splitting Code is a series of articles that give some code that works, and is overall ok, and through a series of steps we work to make it better for readability and maintainability. This series is created to give examples of how to break down large methods so that future development of the code can be easier and quicker.
The code we work with in this first session is a function I added to jsdoc four years ago, 2015, to use a tag’s definition define what container it belongs to. Looking back at the method I understand what the purpose of the code, but it did take me more time than it should to clearly understand its intent.
As nice as JSON is for defining object instances, it does not read well as to what is being done here. We can infer that it is creating an object that has two properties, but what type of object is this? Without breaking out TypeScript to make this code type “safe,” we can describe the intent of the created object by putting it into a helper method. Here we define the helper createContainer
The condition logic we use to determine if a container name is valid is long and disrupts the flow of understanding what is being checked. Extracting this out to a separate helper method with the name containerNameValid helps is understand what is being checked and we will only execute the next line when we have a valid container name.
Getting the container name appears to be simple, but the code that is place is not really clear as to what is going on. To hide the complexity on how to get the container name, I move this code to another helper method getContainerName. Now I know I am getting the container name here, and I don’t need to be concerned about how I am getting.
All the logic going on in the for loop is related to adding a tag’s containers name, provided it is defined and not already in the list. To convey this and know what each iteration the for loop is doing I extract this out to a separate helper method with the descriptive name of addDefinedContainerToContainers.
While reviewing the refactoring, I noticed that I have a variable that is not being used, exceptions. Time to remove the variable to remove any confusion as to why this variable exists.
By creating helper methods that describe what we is the intent and removing a variable that is not used, we leave the code in a cleaner state than it was when we started. Now when I come back to this code four years from now, I will know what the intent more quickly, and make any changes easier.
