8
\$\begingroup\$

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([]);
        })
        
    })
});
\$\endgroup\$
3
\$\begingroup\$

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');
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.