I've just had a new submission in the CFLib queue, and as it's an easy one to test and release, I'm gonna jump on it and get it out there today. However I want to test it first, plus I want to refactor it slightly, so I'm gonna use TDD to do so. And document what I'm doing as I go.
This continues a series on unit testing and TDD that I'm doing, the rest of which are tagged with either "Unit Testing" or "TDD" or both, so you can look them up via those links (there's too many to list now, so I won't bother).
The code for this article will be here: https://github.com/daccfml/scratch/tree/master/cflib/dayOfWeekAsInt (only the baseline files are there at the moment, as I haven't written the code yet ;-).
Simon Bingham has submitted a UDF "dayOfWeekAsInt()" (I'll link to it once I release it), which does what it says on the tin: "Converts a day string to a number (e.g. Sunday to 1, Monday to 2, etc.)". Initially I thought hang on... we have dayOfWeek() already, but Simon's is different in that dayOfWeek() takes a date, and dayOfWeekAsInt() takes a string that is simply the day of the week (in English only, sorry). I think that's reasonably meritorious, so we'll release it.
As I said I need to test it first, and I want to rejig it slightly. First we need baseline tests so that we know when I change things, I don't cause any regressions.
This is the code as submitted:
<!--- dayOfWeekAsInt.cfm --->
<cffunction name="dayOfWeekAsInt" returntype="numeric" output="false" hint="I convert a day string to a number (e.g. Sunday to 1, Monday to 2, etc.)">
<cfargument name="dayAsString" type="string" required="true">
<cfreturn listfindnocase("Sunday,Monday,Tuesday,Wednesday,Thursday,Friday,Saturday", Trim(arguments.dayAsString))>
</cffunction>
I need two baseline tests here: a "happy path" and an "unhappy path". There are two possible outcomes here: one passes a string which returns the integer for the day of the week; one passes a string which doesn't do that (eg: I pass "muppet" to the function).
And here's the first round of tests:
// TestDayOfWeekAsInt.cfc
component extends="mxunit.framework.TestCase" {
public void function beforeTests(){
include "dayOfWeekAsInt.cfm";
variables.daysOfWeek = ["sunday","monday","tuesday","wednesday","thursday","friday","saturday"];
}
public void function baseline(){
for (var i=1; i <= arrayLen(daysOfWeek); i++){
assertEquals(i, dayOfWeekAsInt(daysOfWeek[i]), "Unexpected value returned for #daysOfWeek[i]#");
}
}
public void function badInput(){
var testValue = "BAD_VALUE";
assertFalse(dayOfWeekAsInt(testValue), "'#testValue#' should have returned false");
}
}
Nothing surprising there. dayOfWeekAsInt.cfm just contains the function as shown above.
The first thing I want to refactor is to re-write the thing in script. As I've alluded to in the past ("My CFLib approval process"): in my opinion tags are for views, not code, so I don't do functions in tags. And hey, I'm the approver on CFLib so it's up to me.
In this case there are no test changes, I am just using them to demonstrate I have not caused any regressions with my changes. So the scripted function is this:
// dayOfWeekAsInt.cfm
numeric function dayOfWeekAsInt(required string dayAsString){
return listFindNoCase("Sunday,Monday,Tuesday,Wednesday,Thursday,Friday,Saturday", Trim(arguments.dayAsString));
}
And the tests still pass. Good.
Next: I don't think it's part of the remit of this function to trim() its input. The function is not "trimStringAndGetDayOfWeekAsInt()", so the trim() can go. It'd the job of the calling code to make sure the input is fit for purpose, not the function itself to second-guess the integrity of it. So I need to write a test that will not return a valid result on a space-padded input string:
public void function badPadding(){
var testValue = " Sunday ";
assertFalse(dayOfWeekAsInt(testValue), "'#testValue#' should have returned false");
}
And to make that pass (it currently fails for obvious reasons), we simply get rid of the trim() from the function:
numeric function dayOfWeekAsInt(required string dayAsString){
return listFindNoCase("Sunday,Monday,Tuesday,Wednesday,Thursday,Friday,Saturday", arguments.dayAsString);
}
Another trivial change is I think the argument name is tautological: it's a string, so one doesn't need to mention that in the argument name. Plus we don't need to specifically scope the arg when we use it in this context (see: "Scoping or not scoping?"), so I'll get rid of that.
public void function usingNamedArg(){
dayOfWeekAsInt(day="Monday");
}
The fix:
numeric function dayOfWeekAsInt(required string day){
return listFindNoCase("Sunday,Monday,Tuesday,Wednesday,Thursday,Friday,Saturday", day);
}
And the last change is more significant. Currently the function will "work" with invalid results, in that it will return 0 if the day isn't found in the list. This is incorrect, as there's no 0th day of the week. Also looking at an analogous in-built function, dayOfWeekAsString(), it raises an exception if the input is invalid:
<!--- testDayOfWeekAsString.cfm --->
<cftry>
<cfoutput>#dayOfWeekAsString(0)#</cfoutput>
<cfcatch>
<cfdump var="#[cfcatch.type,cfcatch.message,cfcatch.detail]#">
</cfcatch>
</cftry>
Output:
| Array | |||
| 1 |
| ||
| 2 |
| ||
| 3 |
| ||
I asked around what people thought I should do here - A question in more than 140 characters - and the conclusion was we should raise an exception. So we'll take that approach. We need to adjust one of our tests this time:
/**
* @mxunit:expectedexception expression
*/
public void function badInput(){
dayOfWeekAsInt("BAD_VALUE");
}
And the current state of the function is now thus:
numeric function dayOfWeekAsInt(required string day){
var days = "Sunday,Monday,Tuesday,Wednesday,Thursday,Friday,Saturday";
var index = listFindNoCase(days, day);
if (index){
return index;
}else{
throw(type="expression", message="Invalid day value", details="day argument value must be one of #days#");
}
}
However now we have another unit test breaking:
Which is true, because that test is still expecting a boolean false (ie: zero is false). But we just change that test to expect the exception instead:
/**
* @mxunit:expectedexception expression
*/
public void function badPadding(){
dayOfWeekAsInt(" Sunday ");
}
Oh. One last thing. I'm not that enamoured with abbrevs. in code, so I'm gonna change its name to dayOfWeekAsInteger(). So I'll change all the tests to call that instead, watch them all fail. Annoyingly, badInput() and badPadding() still pass after this:
This is because neither Railo nor ColdFusion are very granular with their approach to what exceptions they raise, and a bad input and a wrong function name both raise "Expression" exceptions. Harrumph. So I'm gonna raise a better exception: ArgumentOutOfRangeException. I've changed those tests to expect one of those exceptions, and now everything fails.
So I change the function to raise the correct exception, and all the tests still fail, which is actually good. They're failing because I've not updated the function name yet. Once I update the function, I get all green, and we're done:
numeric function dayOfWeekAsInteger(required string day){
var days = "Sunday,Monday,Tuesday,Wednesday,Thursday,Friday,Saturday";
var index = listFindNoCase(days, day);
if (index){
return index;
}else{
throw(type="ArgumentOutOfRangeException", message="Invalid day value", details="day argument value must be one of #days#");
}
}
And I'm gonna release that now: dayOfWeekAsInteger(). I hope Simon doesn't disagree with the changes too much!
--
Adam
