Return to Answer

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.

• Style Guide Consider following a style guide. Many common style guides advise separating keywords with a space - e.g. if ( instead of if(.

• Use consistent indentation 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...

• 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. Also, the return statement can eliminate the need for else keywords within a function. One drawback is that calling a function on each iteration may slow down operations but should be negligible for a set of numbers 1 through 100.

Updated Code

Consider the modified code below, utilizing the feedback above. I also used this code in the code contained in my recent question: Fizzbuzz with dynamic height container

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"; }
    if (value % 3 === 0) { return "Fizz"; }
    if (value % 5 === 0) { return "Buzz"; }
    return value;
}
for (var i = 1; i <= 100; i++) {
    console.log(fizzBuzz(i));
}

One option I did consider is to minimize the number of modulo operations, append to the output string instead of outputting it. If you are trying to optimize for speed, this might not be the approach to take because appending to the string may be much slower than doing an extra couple modulo operations. Try a comparison in this jsPerf test.

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

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.

• Style Guide Consider following a style guide. Many common style guides advise separating keywords with a space - e.g. if ( instead of if(.

• Use consistent indentation 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...

• 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. Also, the return statement can eliminate the need for else keywords within a function. One drawback is that calling a function on each iteration may slow down operations but should be negligible for a set of numbers 1 through 100.

Updated Code

Consider the modified code below, utilizing the feedback above. I also used this code in the code contained in my recent question: Fizzbuzz with dynamic height container

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"; }
    if (value % 3 === 0) { return "Fizz"; }
    if (value % 5 === 0) { return "Buzz"; }
    return value;
}
for (let i = 1; i <= 100; i++) {
    console.log(fizzBuzz(i));
}

One option I did consider is to minimize the number of modulo operations, append to the output string instead of outputting it. If you are trying to optimize for speed, this might not be the approach to take because appending to the string may be much slower than doing an extra couple modulo operations. Try a comparison in this jsPerf test.

function fizzBuzz(value) {
    let output = "";
    if (value % 3 === 0) { output += "Fizz"; }
    if (value % 5 === 0) { output += "Buzz"; }
    return output || value;
}
for (let i = 1; i <= 100; i++) {
    console.log(fizzBuzz(i));
}

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.

• Style Guide Consider following a style guide. Many common style guides advise separating keywords with a space - e.g. if ( instead of if(.

• Use consistent indentation 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...

• 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. Also, the return statement can eliminate the need for else keywords within a function. One drawback is that calling a function on each iteration may slow down operations but should be negligible for a set of numbers 1 through 100.

###Updated Code Consider the modified code below, utilizing the feedback above. I also used this code in the code contained in my recent question: Fizzbuzz with dynamic height container

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"; }
    if (value % 3 === 0) { return "Fizz"; }
    if (value % 5 === 0) { return "Buzz"; }
    return value;
}
for (var i = 1; i <= 100; i++) {
    console.log(fizzBuzz(i));
}

One option I did consider is to minimize the number of modulo operations, append to the output string instead of outputting it. If you are trying to optimize for speed, this might not be the approach to take because appending to the string may be much slower than doing an extra couple modulo operations. Try a comparison in this jsPerf test.

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

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.

• Style Guide Consider following a style guide. Many common style guides advise separating keywords with a space - e.g. if ( instead of if(.

• Use consistent indentation 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...

• 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. Also, the return statement can eliminate the need for else keywords within a function. One drawback is that calling a function on each iteration may slow down operations but should be negligible for a set of numbers 1 through 100.

Updated Code

Consider the modified code below, utilizing the feedback above. I also used this code in the code contained in my recent question: Fizzbuzz with dynamic height container

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"; }
    if (value % 3 === 0) { return "Fizz"; }
    if (value % 5 === 0) { return "Buzz"; }
    return value;
}
for (var i = 1; i <= 100; i++) {
    console.log(fizzBuzz(i));
}

One option I did consider is to minimize the number of modulo operations, append to the output string instead of outputting it. If you are trying to optimize for speed, this might not be the approach to take because appending to the string may be much slower than doing an extra couple modulo operations. Try a comparison in this jsPerf test.

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

• use strict equality comparison - i.e. === when comparing values. That way it won't need to convert the types.
• Style Guide Consider following a style guide. Many common style guides advise separating keywords with a space - e.g. if ( instead of if(.
• Use consistent indentation 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 - If you are going to use block statements then expand them so each statement is on a separate line. Otherwise, just put the single statement without a block. 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.
• 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. Also, the return statement can eliminate the need for else keywords within a function. One drawback is that calling a function on each iteration may slow down operations but should be negligible for a set of numbers 1 through 100.

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

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

• use strict equality comparison - i.e. === when comparing values. That way it won't need to convert the types.

• Style Guide Consider following a style guide. Many common style guides advise separating keywords with a space - e.g. if ( instead of if(.

• Use consistent indentation 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...

• 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. Also, the return statement can eliminate the need for else keywords within a function. One drawback is that calling a function on each iteration may slow down operations but should be negligible for a set of numbers 1 through 100.

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

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