Caffeine-Driven FizzBuzz

I have been given following problem to solve:

• If the integer is divisible by 3, return the string "Java".

• If the integer is divisible by 3 and divisible by 4, return the string "Coffee"

• If the integer is one of the above and is even, add "Script" to the

end of the string. Otherwise, return the string "mocha_missing!"

Sample Input and Output

• caffeineBuzz(1) => "mocha_missing"

• caffeineBuzz(3) => "Java"

• caffeineBuzz(6) => "JavaScript"

• caffeineBuzz(12) => "CoffeeScript"

//Class 

    public class CoffieScriptGenerator {

        public String caffeineBuzzz(Integer number) {

            if(isDivisbleByThreeAndFour(number))
            {
                if(isEven(number))
                return "CoffeeScript";
                else
                return "Coffee";
            }
            if(isDivisbleByThree(number))
            {
                if(isEven(number))
                return "JavaScript";
                else
                return "Java";
            }
            return "mocha_missing!";

        }

        private boolean isEven(Integer number) {
            return number%2==0; 
        }

        private boolean isDivisbleByThreeAndFour(Integer number) {

            return isDivisbleByThree(number) && number%4==0;
        }

        private boolean isDivisbleByThree(Integer number) {
            return number%3==0;
        }

    }

Please provide suggestions to refactor it and improve it and how i can improve such nesting if else with help of patterns like strategy.

Your private one-liner functions do very little to help with anything, and they're not really DRY. Might as well do this:

private boolean isDivisibleBy(Integer number, Integer divideBy) {
    return number % divideBy == 0;
}

As you can see this does nothing much more than wrap the modulo operator with a function call... which is questionable. Notice:

• "Divisible" - not sure why your functions lost an i in the process.

• Spacing. There's no need to cram operators and operands together into as little horizontal spaces as possible - let code breathe. Especially if the reason for having these functions in the first place, is to improve readability!

Your code doesn't comply with the specs: it's not adding "Script" to the end of the string when the number is even. In fact, your logic is redundant:

if(isDivisbleByThreeAndFour(number)) {
    ...
}

if(isDivisbleByThree(number)) {
    ...
}

Notice:

• Both blocks check if number can be divided by 3.

• Consistent brace position. If you're going to put scope-opening braces at the end of the line, keep 'em that way - don't switch to C#-style next-line bracing in the middle of your code!

Your bracing style not only inconsistent, the braces themselves are inconsistently present!

if(isEven(number))
    return "CoffeeScript";
    else
    return "Coffee";

Don't do that. Use braces, and indentation. For your own health.

if(isEven(number)) {
    return "CoffeeScript";
} else {
    return "Coffee";
}

Note, when you have if (x) { return foo; } else { return bar; }, it can be rewritten with a ternary operator, like so:

return x ? foo : bar;

But you don't want to do that here.

You don't want that, because you'll need nested ternaries to remove the redundancies, and that would look terrible.

What would the correct logic be then?

Just follow the specs:

• Check to see if it divides by 3 - if it does:

• If it also divides by 4, we got "Coffee". Else, we got "Java".

• If it also divides by 2, append "Script" to whatever we got.

• Otherwise, return "mocha_missing!"

The nesting here should give you a clue: you only need to check once if the number can divide by 3.

String value;
if (number % 3 == 0) {
    if (number % 4 == 0) {
        value = "Coffee";
    } else {
        value = "Java";
    }
    if (number % 2 == 0) {
        value += "Script";
    }
} else {
    value = "mocha_missing!";
}

return value;

Strategy Pattern seems very much overkill here. If this is for an interview, I'd say don't overthink it - they're just trying to see if you can read specs.

StackExchange Code Review Q#126035, answer score: 19