Code Review Asked on December 19, 2021
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([]);
})
})
});
Instead of returning an array when inputs are invalid, throw exceptions and use throw
to detect it in the tests.
else
when previous blocks have return statementsThe 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).
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');
Answered by Sᴀᴍ Onᴇᴌᴀ on December 19, 2021
Get help from others!
Recent Questions
Recent Answers
© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP