Code Kata Walkthrough - Part III
We left off last post with a string calculator that could sum up numbers separated by a new line or a comma. We also did our first refactoring phase where we cleaned up our code without actually changing the functionality. Let’s take a look at the next test case.
The next test case that I am going to write will include a list of numbers that are separated by both commas and new lines. The kata specifically gives this example: “1\n2,3” should equal 6. Let’s turn that into code:
it("should return the sum of three numbers when passed three numbers separated by a new line and a comma", function() {
expect(calc.add("1\n2,3")).toBe(6);
});
If we run the test suite, we see that this test actually passes without us needing to make any code changes. We are still in the GREEN state. During our REFACTOR phase,
we decided to consolidate our duplicated code into one if
statement that checks for both commas and new lines. We then decided to use a regular expression so that our
code would split the input on either commas or new lines. By taking our time to go back and refactor our code, we were able to pass a test case without writing a single
line of new code.
Onto the next test case. The kata’s next requirement is to support different delimiters, which can be specified at the beginning of the string in the following format: “//[delimiter]\n[numbers]”. Let’s write a test case using the semicolon as a delimiter.
it("should return the sum of two numbers when passed two numbers separated by the delimeter specified", function() {
expect(calc.add("//;\n2;5;3")).toBe(10);
});
This puts us in the RED state. Let’s implement some code to handle different delimiters.
As I looked at how I might implement the code for the next test case, I realized that it would be easier if I switched some things around. What I want to do is refactor my current code before I start writing new code. Since we added this new test case, we are in the RED state. As I said in the last post, the biggest rule of refactoring is ALWAYS REFACTOR IN THE GREEN STATE. So what I am going to do is comment out the new test case that I just added like so:
/*it("should return the sum of two numbers when passed two numbers separated by the delimeter specified", function() {
expect(calc.add("//;\n2;5;3")).toBe(10);
});*/
This puts us back in the GREEN state. Now I can refactor my code and execute my tests during the process to ensure that my refactoring has not broken anything. After refactoring, the code looks like this:
Calculator.prototype.add = function(str) {
if(str.length == 0) {
return 0;
} else if(str.length == 1) {
return parseInt(str);
} else {
nums = str.split(/[,\n]/);
sum = 0;
for (i = 0; i < nums.length; i++) {
sum += parseInt(nums[i]);
}
return sum;
}
}
All I did was switch around the else
case and the else if
case. This will allow me implement the code that I have in mind for the next test case more easily. Now that
we have finished refactoring, let’s uncomment our test case to get us back in the RED state. And now let’s implement the code to get us back in the GREEN state.
Calculator.prototype.add = function(str) {
if(str.length == 0) {
return 0;
} else if(str.length == 1) {
return parseInt(str);
} else {
if(str.startsWith("//")) {
delim = str.substr(2, str.indexOf("\n")-2);
nums = str.substring(str.indexOf("\n")+1).split(delim);
} else {
nums = str.split(/[,\n]/);
}
sum = 0;
for (i = 0; i < nums.length; i++) {
sum += parseInt(nums[i]);
}
return sum;
}
}
So we check if the user is passing in a customer delimiter by looking for “//”. If we find that, then we do some string manipulation to extract the delimiter and the list of
numbers. Now let’s think REFACTOR. This code looks pretty ugly. At the very least, we could extract some of those substrings out into variables so it is more readable. We
are also calling nums = str.split
twice. Let’s start with just these things and see where we get.
Remember, that even though you do not see it here, I am executing the tests after every change that I make during the REFACTOR phase to ensure that I am always in the GREEN state. During this refactor, I actually ended up down a rabbit hole in the RED state, so I decided to CTRL-Z my way all the way back to exactly how I have the code up there, and I tried again. If you get stuck in the RED, undo until you are back in the GREEN. This is what I came up with:
Calculator.prototype.add = function(str) {
if(str.length == 0) {
return 0;
} else if(str.length == 1) {
return parseInt(str);
} else {
delim = "";
if(str.startsWith("//")) {
delimLen = str.indexOf("\n")-2;
numsStartPos = str.indexOf("\n")+1;
delim = str.substr(2, delimLen);
str = str.substring(numsStartPos);
}
rgx = new RegExp("[" + delim + ",\n]");
nums = str.split(rgx);
sum = 0;
for (i = 0; i < nums.length; i++) {
sum += parseInt(nums[i]);
}
return sum;
}
}
Now this might be more readable, but it is getting to be a pretty long function. We are doing a lot of logic to extract the delimiter and split the numbers lists. Maybe we can pull this out into it’s own method. Let’s give it a shot.
Calculator.prototype.add = function(str) {
if(str.length == 0) {
return 0;
} else if(str.length == 1) {
return parseInt(str);
} else {
nums = getNumsList(str);
sum = 0;
for (i = 0; i < nums.length; i++) {
sum += parseInt(nums[i]);
}
return sum;
}
}
getNumsList = function(str) {
delim = "";
if(str.startsWith("//")) {
delimLen = str.indexOf("\n")-2;
numsStartPos = str.indexOf("\n")+1;
delim = str.substr(2, delimLen);
str = str.substring(numsStartPos);
}
rgx = new RegExp("[" + delim + ",\n]");
return str.split(rgx);
}
Now that looks much better. And we got there with very little stress because if we screwed up, we were always able to get back into a stable state and start again. We’ll continue with this in the next post!