Code Kata Walkthrough - Part IV

We left off the last post on a good refactoring session. Let’s take a look at the next kata requirement, which is to throw an exception when a negative number is passed in the list. The exception should include the negative number. If there are multiple negative numbers, then it should include all the numbers.

We will start with the case of only one negative number. The test case looks like this:

it("should throw an exception and the number if a negative number is passed", function() {
    expect(function() {calc.add("//;\n2;-5;3")}).toThrowError("Negative numbers not allowed: -5");
});

This puts us back in the RED state since we have no code to treat negative numbers any differently than positive numbers. Let’s fix this:

Calculator.prototype.add = function(str) {
    if(str.length == 0) {
        return 0;
    } else if(str.length == 1) {
        return parseInt(str);
    } else {
        nums = getNumsList(str);
        for(i = 0; i < nums.length; i++) {
            if(nums[i] < 0) {
                throw new Error("Negative numbers not allowed: " + nums[i]);
            }
        }
        sum = 0;
        for (i = 0; i < nums.length; i++) {
            sum += parseInt(nums[i]);
        }
        return sum;
    }
}

We add a loop to check all the numbers and throw an exception on the first negative that it finds. We’re back in the GREEN state. Let’s take a look to see if we have the opportunity to refactor anywhere. The error checking logic doesn’t seem like it is in the right place. In order to keep our add method nice and clean, we should move the error checking logic out into it’s own method.

Calculator.prototype.add = function(str) {
    if(str.length == 0) {
        return 0;
    } else if(str.length == 1) {
        return parseInt(str);
    } else {
        nums = getNumsList(str);
        checkForNegatives(nums);
        
        sum = 0;
        for (i = 0; i < nums.length; i++) {
            sum += parseInt(nums[i]);
        }
        return sum;
    }
}

checkForNegatives = function(nums) {
    for(i = 0; i < nums.length; i++) {
        if(nums[i] < 0) {
            throw new Error("Negative numbers not allowed: " + nums[i]);
        }
    }
}

Looks good so far. Let’s add a test case for multiple negative numbers now.

it("should throw an exception and the list of numbers if multiple negative numbers are passed", function() {
    expect(function() {calc.add("//;\n-2;-5;-3;5")}).toThrowError("Negative numbers not allowed: -2, -5, -3");
});

This is failing because we throw an exception as soon as we find the first negative number. Let’s fix that by altering our checkForNegatives method.

checkForNegatives = function(nums) {
    errMsg = "Negative numbers not allowed: "
    errFound = false;
    
    for(i = 0; i < nums.length; i++) {
        if(nums[i] < 0) {
            errMsg = errMsg + nums[i] + ", ";
            errFound = true;
        }
    }
    
    if(errFound) {
        errMsg = errMsg.slice(0, -2); // Remove trailing comma
        throw new Error(errMsg);
    }
}

That puts us back in the GREEN. Do we need to refactor? This looks pretty good to me. I think we can move on to the next test case. The next requirement is that numbers greater than 1000 should be ignored. Here is our test case:

it("should ignore numbers greater than 1000", function() {
    expect(calc.add("5,1001\n6,5126")).toBe(11);
});

Let’s get this test case passing by altering our getNumsList method.

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]");
    nums = str.split(rgx);
    for(i = 0; i < nums.length; i++) {
        if(nums[i] > 1000) {
            nums.splice(i, 1);
        }
    }
    return nums;
}

We’re back in the GREEN. Let’s consider some refactoring. I think we can move the logic to remove numbers over 1000 into its own method.

removeNumsOver1000 = function(nums) {
    for(i = 0; i < nums.length; i++) {
        if(nums[i] > 1000) {
            nums.splice(i, 1);
        }
    }
    
    return nums;
}

That’s all I’ll do in this post. I think we’ve got the feel for it at this point. I’ll finish up the last couple requirements for the kata and finish with some closing thoughts in the next post.

Written on November 9, 2017