The Scripted FizzBuzz Kata

I run a coding dojo at work. for one session I'm to showing that you can use a kata to get into a new language.

I'm using the FizzBuzz Kata to show JavaScript (and F#) because it is short. The problem is my JavaScript knowledge is only slightly above the copy -> paste -> fiddle level.

Thus, I would like to see better ways of doing this (using qunit):

var fizzBuzzer = (function () {
var For = function (number) {
var isBuzz = number % 5 === 0;
var isFizz = number % 3 === 0;

if ((isFizz) && (isBuzz)) {return "FizzBuzz";}
if (isBuzz ) {return "Buzz";}
if (isFizz ) {return "Fizz";}

return number;
};

return { For: For };
})();

test("not a multiple of 5 or 3 returns number", function () {
equal(fizzBuzzer.For(1), 1);
equal(fizzBuzzer.For(2), 2);
});

test("multiple of 3 returns Fizz", function () {
equal(fizzBuzzer.For(3), "Fizz");
equal(fizzBuzzer.For(6), "Fizz");
});

test("multiple of 5 returns Buzz", function () {
equal(fizzBuzzer.For(5), "Buzz");
equal(fizzBuzzer.For(10), "Buzz");
});

test("multiple of 3 and 5 returns FizzBuzz", function () {
equal(fizzBuzzer.For(15), "FizzBuzz");
equal(fizzBuzzer.For(30), "FizzBuzz");
});

• Related, the F# version (No duplicate) : codereview.stackexchange.com/questions/64871/kata-f-fizzbuzz – Marc-Andre Oct 6 '14 at 13:48
• If this is for a kata, your only responsibility to write the tests - your own implementation isn't important. The actual implementation is completely up to the student; it just has to pass the tests. For instance, I'd write the function very differently, but that doesn't really matter. So follow tim's advice and add some more tests - and skip the unnecessary and confusing For-thing. – Flambino Oct 6 '14 at 19:14
• fizzBuzzer should be getFizzBuzzClassification. Describe what the function does. – usr Oct 6 '14 at 21:14

Structure

I'm not quite sure why you chose to write your program the way you did. It seems to add complexity to the naive approach of using a normal function, without adding any benefits:

function fizzBuzzer(number) {
var isBuzz = (number % 5) === 0;
var isFizz = (number % 3) === 0;

if (isFizz && isBuzz) { return "FizzBuzz"; }
if (isBuzz) { return "Buzz"; }
if (isFizz) { return "Fizz"; }

return number;
}


Misc

• parentheses: sometimes you have too many (for example ((isFizz) && (isBuzz)) would be more readable as (isFizz && isBuzz)), and sometimes you could use a bit more (for example, to understand var isBuzz = number % 5 === 0;, you need to remember the precedent of 3 things; I would probably change it to var isBuzz = (number % 5) === 0;).
• spaces: I wouldn't put a space before ). But I would put one after { and before } to make the code more readable. So if (isBuzz ) {return "Buzz";} for example would become if (isBuzz) { return "Buzz"; }.
• I like that you wrote unit tests for JavaScript code. You could also include a test for 0, and maybe negative numbers (as they are allowed as input to your function).
• It should also be noted, that For (written with PascalCase) implies that the function is a constructor - if anything it should be for (except that's a reserved word, of course). So yeah, I agree that the current structure is confusing and unnecessary. – Flambino Oct 6 '14 at 19:12

I agree with @Tim's points, though some of the code formatting is more subjective.

You don't have a check to make sure that it is a number. While that is not a strict requirement of the task, input validation is something worth doing. After all, if the user enters 'Fizz' as an input, your program will output 'Fizz'.

Personally, any time I can get away with easy type coercion I do. So instead of having strict equality with 0, I'll just have the following:

isFizz = !(number % 5)


While this is not strictly necessary, it is worth considering. It is faster, shorter, and it demonstrates a knowledge of the idea of "truthy" vs. "falsey" in JavaScript.