I am currently getting closely acquainted with Behavior-Driven Development. Could someone tell me how I am doing with the Fizzbuzz program below? I am interested in both improving the JavaScript code and the BDD prowess. Any help is appreciated.
fizzbuzz.js
module.exports = function(word, value){
function sortNumber(a,b) {
return a - b;
}
numbers = function(divisor, max){
var result = []
for(i=1;i<=max;i++){
if ((i % divisor) === 0)
result.push(i)
}
return result
}
if (value <= 0)
return ['error','nonpositive']
else {
switch(word) {
case 'fizz':
return numbers(3, value)
case 'buzz':
return numbers(5,value)
case 'fizzbuzz':
return numbers(3,value).concat(numbers(5,value)).filter(function(elem,index,self){
return index == self.indexOf(elem)
}).sort(sortNumber)
default:
return ['error','wrongword']
}
}
};
test/fizzbuzz-spec.js:
var chai = require('chai');
expect = chai.expect,
fizzbuzz = require('../lib/fizzbuzz');
describe('A fizzbuzz', function(){
describe('will return an error for invalid words: ', function(){
it('like fuss', function(){
expect(fizzbuzz('fuss', 10)).to.be.deep.equal(['error','wrongword']);
});
it('like biss', function(){
expect(fizzbuzz('biss', 10)).to.be.deep.equal(['error','wrongword']);
});
});
describe('will return an error when non-positive integer is passed', function(){
it('like 0', function(){
expect(fizzbuzz('buzz', 0)).to.be.deep.equal(['error','nonpositive']);
})
it('like -2', function(){
expect(fizzbuzz('buzz', -2)).to.be.deep.equal(['error','nonpositive']);
})
})
describe('will return valid non-error input when positive integers and valid words are passed', function(){
it('like fizz and 20', function(){
expect(fizzbuzz('fizz', 20)).not.to.contain('error');
})
it('like buzz and 20', function(){
expect(fizzbuzz('buzz', 20)).not.to.contain('error');
})
it('like fizzbuzz and 20', function(){
expect(fizzbuzz('fizzbuzz', 20)).not.to.contain('error');
})
})
describe('will return valid results for some basic valid test cases', function(){
it('like fizz and 10', function(){
expect(fizzbuzz('fizz', 10)).to.be.deep.equal([3,6,9]);
})
it('like fizz and 20', function(){
expect(fizzbuzz('fizz', 20)).to.be.deep.equal([3,6,9,12,15,18]);
})
it('like buzz and 10', function(){
expect(fizzbuzz('buzz', 10)).to.be.deep.equal([5,10]);
})
it('like buzz and 25', function(){
expect(fizzbuzz('buzz', 25)).to.be.deep.equal([5,10,15,20,25]);
})
it('like fizzbuzz and 10', function(){
expect(fizzbuzz('fizzbuzz', 10)).to.be.deep.equal([3,5,6,9,10]);
})
it('like fizzbuzz and 25', function(){
expect(fizzbuzz('fizzbuzz', 25)).to.be.deep.equal([3,5,6,9,10,12,15,18,20,21,24,25]);
})
it('like fizz and 2', function(){
expect(fizzbuzz('fizz', 2)).to.be.deep.equal([]);
})
it('like buzz and 4', function(){
expect(fizzbuzz('buzz', 4)).to.be.deep.equal([]);
})
})
});
1 Answer 1
Handling invalid input
Instead of returning an array when inputs are invalid, throw exceptions and use throw
to detect it in the tests.
Avoid else
when previous blocks have return statements
The else
can be eliminated since the block evaluated when value <= 0
has a return statement (or would throw
an exception). Additionally, the switch
statement can be simplified to a few if
statements (for an example see this answer to a related question).
Don't forget to set scopes
While the code is in a module, it is best to set the scope of variables and functions - e.g. numbers
and i
in the loop within numbers
should be prefixed by the keywords const
and let
, respectively, to avoid accidental re-assignment/confusion.
In that same vein, the first few lines of the test are setting variables:
var chai = require('chai'); expect = chai.expect, fizzbuzz = require('../lib/fizzbuzz');
Perhaps you meant to add a comma instead of semi-colon after require('chai')
? It would be simpler just to have each variable on its own line:
const chai = require('chai');
const expect = chai.expect;
const fizzbuzz = require('../lib/fizzbuzz');
Actually instead of importing chai
just to use expect
from it on a separate line, use object-destructuring:
const { expect } = require('chai');
const fizzbuzz = require('../lib/fizzbuzz');
Explore related questions
See similar questions with these tags.