Last week (I think) whilst I was messing around with some code, and TDDing it as I went, I posted on Twitter about my pleasure at testing with TestBox which yielded a request from Dan Fredericks:
@dacCfml @bdw429s @lmajano you have lots of examples of testbox test on your blog or github, i need to see more real world examples, pls :)The code I was working on will make it onto the blog at some point, but in the interim I was simplifying a CFLib UDF yesterday, and needed some regression tests for my fix, so took the TDD approach. And here's the business.
— Dan Fredericks (@fmdano74) July 3, 2014
Firstly, here's the original UDF:
function FirstXDayOfMonth(day_number,month_number,year) {
var start_of_month = CreateDate(year,month_number,1); // date object of first day for given month/year
var daydiff = DayOfWeek(start_of_month) - day_number;
var return_date = "";
switch(daydiff) {
case "-1": return_date = DateAdd("d",1,start_of_month); break;
case "6": return_date = DateAdd("d",1,start_of_month); break;
case "-2": return_date = DateAdd("d",2,start_of_month); break;
case "5": return_date = DateAdd("d",2,start_of_month); break;
case "-3": return_date = DateAdd("d",3,start_of_month); break;
case "4": return_date = DateAdd("d",3,start_of_month); break;
case "-4": return_date = DateAdd("d",4,start_of_month); break;
case "3": return_date = DateAdd("d",4,start_of_month); break;
case "-5": return_date = DateAdd("d",5,start_of_month); break;
case "2": return_date = DateAdd("d",5,start_of_month); break;
case "-6": return_date = DateAdd("d",6,start_of_month); break;
case "1": return_date = DateAdd("d",6,start_of_month); break;
default: return_date = start_of_month; break; // daydiff=0, default to first day in current month
} //end switch
return return_date;
}
I looked at that and thought "that's an awful lot of code to do something so simple". I also looked at that bloody great
switch statement and its mishmash of hard-coded data / magic numbers and code and creased my brow a bit.Working out the first specific day of the month is an algorithm, not a data-look-up exercise!
I hastily worked out a sketchy algorithm in my head: "it's be something to do with taking the difference between the day-of-week of the first of the month, and day-of-week specified in the arguments, and adding them to the first of the month. And there'll be some monkeying with it if the argument is lower than the day-of-week of the first of the month". That's as fleshed out as I got it... I knew it would be a simple one once I worked it out, but in the mean time I needed some regression tests. Both to prove the existing function works, and that my replacement for it also works.
I settled on these tests:
- when the specified day-of-week (DOW) is more than the DOW of the first of the month (DOW1st);
- when DOW is less than DOW1st;
- when DOW is the same as DOW1st;
- when DOW is one less than DOW1st;
- when DOW is one more than DOW1st;
- when DOW is 1 and DOW1st is 7;
- when DOW is 7 and DOW1st is 1;
- when DOW is less than 1;
- when DOW is greater than 7.
- baseline tests - just some general testing;
- boundary tests - situations in which the inputs at at or adjacent to situations which cause the algorithm to behave differently;
- exception tests - how invalid situations are handled.
//TestFirstXDayOfMonth.cfc
component extends="testbox.system.BaseSpec" {
function beforeAll(){
include "udfs/firstXDayOfMonth.cfm";
variables.firstDayOfMonthIs = {
1 = createDate(2014,6,1),
2 = createDate(2014,9,1),
3 = createDate(2014,4,1),
4 = createDate(2014,10,1),
5 = createDate(2014,5,1),
6 = createDate(2014,8,1),
7 = createDate(2014,2,1)
};
}
function run(){
describe("Tests for firstXDayOfMonth()", function(){
describe("edge cases", function(){
it("works in a general case wherein the requested DOW is after the DOW of the first of the month", function(){
var testDate = firstDayOfMonthIs[2];
expect(
firstXDayOfMonth(4, month(testDate), year(testDate))
).toBe(dateAdd("d", 4-2, testDate));
});
it("works in a general case wherein the requested DOW is before the DOW of the first of the month", function(){
var testDate = firstDayOfMonthIs[6];
expect(
firstXDayOfMonth(4, month(testDate), year(testDate))
).toBe(dateAdd("d", 7-(6-4), testDate));
});
});
describe("edge cases", function(){
it("works with the same DOW as the first of the month", function(){
var testDate = firstDayOfMonthIs[3];
expect(
firstXDayOfMonth(3, month(testDate), year(testDate))
).toBe(testDate);
});
it("works with a DOW immediately before the first of the month's one", function(){
var testDate = firstDayOfMonthIs[4];
expect(
firstXDayOfMonth(3, month(testDate), year(testDate))
).toBe(dateAdd("d", 7-(4-3), testDate));
});
it("works with a DOW immediately after the first of the month", function(){
var testDate = firstDayOfMonthIs[5];
expect(
firstXDayOfMonth(6, month(testDate), year(testDate))
).toBe(dateAdd("d", 6-5, testDate));
});
it("works with 1 as request DOW and 7 as the 1st's DOW", function(){
var testDate = firstDayOfMonthIs[7];
expect(
firstXDayOfMonth(1, month(testDate), year(testDate))
).toBe(dateAdd("d", 7-(7-1), testDate));
});
it("works with 7 as request DOW and 1 as the 1st's DOW", function(){
var testDate = firstDayOfMonthIs[1];
expect(
firstXDayOfMonth(7, month(testDate), year(testDate))
).toBe(dateAdd("d", 7-1, testDate));
});
});
describe("invalid cases", function(){
it("errors if passed 0 as the requested DOW", function(){
expect(function(){
firstXDayOfMonth(0, 1, 1);
}).toThrow("InvalidDayOfWeekException");
});
it("errors if passed 8 as the requested DOW", function(){
expect(function(){
firstXDayOfMonth(8, 1, 1);
}).toThrow("InvalidDayOfWeekException");
});
});
});
}
}
A lot of that is repetition, but there's a coupla things to look at:
- I created a series of test dates which I confirmed the validity of, and used those in my tests.
- The rule when the requested DOW is after the DOW of the 1st, then simply add the difference, and that's your date.
- If the requested DOW is before the DOW of the 1st, then subtract that difference from 7. This is because the first date with that DOW must intrinsically be in the following week (eg: the month started on a Weds... the first Tues is going to be the following week, not that week).
- I decided on an arbitrary exception to check for if the requested DOW was outside 1-7, which are the only valid values here.
The invalid case tests were not going to pass, as I was looking for a specific custom exception which the existing function did not. Note the existing function does not reject these invalid values at all, instead allows one to work out what the first - for example - 9th day of the week is. Which is obvious nonsense.
OK, so armed with working tests, I set about refactoring the function. I'm safe to do so because I've got close enough to 100% test coverage here, so I know if I do anything wrong, the tests will catch it. Also in the process of writing the tests, I have worked out the algorithm I need to use in my code. TDD at work. Here's the new function:
date function firstXDayOfMonth(required numeric dayOfWeek, required numeric month, required numeric year){
if (dayOfWeek < 1 || dayOfWeek > 7){
throw(type="InvalidDayOfWeekException", message="Invalid day of week value", detail="the dayOfWeek argument must be between 1-7 (inclusive).");
}
var firstOfMonth = createDate(year, month, 1);
var dowOfFirst = dayOfWeek(firstOfMonth);
var daysToAdd = (7 - (dowOfFirst - dayOfWeek)) MOD 7;
var dow = dateAdd("d", daysToAdd, firstOfMonth);
return dow;
}
The only slightly unexpected thing here is the
MOD, but this just caters for the situation in which dowOfFirst and dayOfWeek are the same, which otherwise would result in (7 - (dowOfFirst - dayOfWeek)) equalling 7, when it should equal 0. I got bitten by this on my first pass at writing the function, but the tests demonstrated where I was wrong straight away, so the fix was easy to work out. TDD wins out again!I think we can agree that that is much more concise, and just gets on with it instead of unnecessary faffing around. I could have ditched some of the intermediary variables there, but I'd rather err towards readable, understandable code than brevity for the sake of a few keystrokes in the code or bytes in memory.
You might ask - if I'm validating
dayOfWeek to fall within 1-7 - why I do not do the same with the month and year values. This is simply because dayOfWeek is only used in my own logic, whilst the other two are passed to CFML functions which will themselves validate their appropriateness: createDate() will fail if month and year are invalid. So I only need to validate the inputs into my own logic. There would be a case for me to validate these other two as well, but I thought the fallback situation (and the errors CF raises) are clear enough, and the code is short and open source, so easy enough to troubleshoot.There is one other thing that's comment-worthy here as it's the second time I have had to deal with this sort of thing this week, in two different situations.
If we take the
switch block in the original function in isolation, it is simply not the right way to deal with the logic requirement. Each case is identical except for the data values being used: we're mapping one value onto another value. If this is the necessity... use a map! Here's a version of that function using a map instead of code/data mishmash:function firstXDayOfMonthMap(day_number,month_number,year) {
if (day_number < 1 || day_number > 7){
throw(type="InvalidDayOfWeekException", message="Invalid day of week value", detail="the dayOfWeek argument must be between 1-7 (inclusive).");
}
var start_of_month = createDate(year,month_number,1); // date object of first day for given month/year
var daydiff = dayOfWeek(start_of_month) - day_number;
var transformationMap = {
"-1" = 1, 6 = 1,
"-2" = 2, 5 = 2,
"-3" = 3, 4 = 3,
"-4" = 4, 3 = 4,
"-5" = 5, 2 = 5,
"-6" = 6, 1 = 6,
0 = 0
};
return dateAdd("d", transformationMap[daydiff], start_of_month);
}
Now that's enough procrastination. I have a job interview tomorrow I need to prep for. I need to give a presentation :-S. I do not do well giving presentations. Bleah.
--
Adam
