Return to Revisions

I'd like to see if this is a reasonable solution, or is just plain terrible.

I wouldn't say it is "terrible" - mostly because it works and doesn't appear to be very inefficient. However, there are some improvements that can be made.

• use strict equality comparison - i.e. === when comparing values. That way it won't need to convert the types.
• Use consistent indentation for readability The first and last lines within the for loop are not indented, while the other lines between them are, though maybe it was the case that your code was indented properly but when you pasted here it was thrown off because of the markdown formatting...
• Statements and blocks - For the sake of readability, expand blocks so each statement is on a separate line. Also use consistent statement termination. The first three conditional blocks don't have a semi-colon after the statement within the block, while the last one (in the else block) does.
• Consider default value output is never left as "" because it is always set in one of the if, else if or else blocks. The else could be eliminated if output is assigned to i. In a strongly typed language this might have ramifications but not in JavaScript.
• Abstract logic into a function As Paul's answer suggests: you can put the core logic into a function that returns the output but doesn't handle outputting the value (e.g. to the console). This allows such code to be atomic and testable - congruent with the Single Responsibility Principle.

###Updated Code Consider the modified code below, utilizing the feedback above.

Note: the inline console in the snippets is truncated to ~50 lines, but the complete console log should be visible in your browser console.

function fizzBuzz(value) {
    if (value % 15 === 0) {
        return "FizzBuzz";
    }
    else if (value % 3 === 0) {
        return "Fizz";
    }
    else if (value % 5 === 0) {
        return "Buzz";
    }
    return value;
}
for (var i = 1; i <= 100; i++) {
    console.log(fizzBuzz(i));
}