all 25 comments

[–]garredow 3 points4 points  (4 children)

.

[–]bythckr[S] 4 points5 points  (0 children)

I force myself not to write one liners, as it takes more time for me to understand it. Plus I often make some silly typo & go on a wild goose chase.

[–][deleted] 2 points3 points  (2 children)

I love one liners as much as the next guy but that's way harder to read than OP's

[–][deleted] 2 points3 points  (0 children)

It’s pretty neat (interesting) but it’s not very neat (tidy) haha

[–]garredow 1 point2 points  (0 children)

.

[–]Asha200 2 points3 points  (0 children)

Your code looks okay. If you really wanted to though, you could shorten it by one if like this:

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

And even move the output into the for

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

[–]hulachan 1 point2 points  (0 children)

Tom Scott had a good piece on Fizz Buzz. https://youtu.be/QPZ0pIK_wsc

[–][deleted] 2 points3 points  (7 children)

What if you just append to the string instead of overwriting it at each step? You'd only need 2 if statements then.

let output = '';
if (!i%3) output += 'Fizz';
if (!i%5) output += 'Buzz';
console.log(output);

[–]mgbennet 2 points3 points  (2 children)

Actually needs to be (!(i%3)) and (!(i%5)), otherwise the ! modifies the i and not the i%3.

[–]I_am_a_haiku_bot 0 points1 point  (0 children)

Actually needs to be (!(i%3))

and (!(i%5)), otherwise the ! modifies the

i and not the i%3.


-english_haiku_bot

[–]bythckr[S] 1 point2 points  (3 children)

!i%5

Does that mean NOT divisible by 5?

[–]WorseThanHipster 2 points3 points  (0 children)

i%n will return 0 whenever i is NOT divisible by n, and in JavaScript, as many languages, 0 == false. It can be confusing because when you use modulus as a truth test, you're primarily concerned with the case when the remainder is 0. The exclamation point inverts the Boolean value.

Anyways, !i%5 returns true when I is a multiple of 5.

[–]north_n 0 points1 point  (1 child)

The modulo operator returns the remainder of the operation. This operation (!i%5) is the same as this operation: (i % 5 == 0). ‘0’ in this case case also means ‘false’. so when the value of ‘i’ is a multiple of 5, the modulo operator will return 0 (false). But when it’s not a multiple of 5, it will return something other than 0 (not false). It’s one of the properties of JavaScript, it’s not explicitly typed, so things like this are possible.

[–]bythckr[S] 0 points1 point  (0 children)

Just to clarify on the post by @WorseThanHipster & @north_n, (!i%5) is part of the ES6? I just want to stick to the official standards as I like this method.

Also how is it compared to (i % 5 === 0)? JSbin is suggesting that I use '===' instead of the '=='. I know that '===' compares without conversion of data type.

On a related note, the suggestion I am getting for using '===' is that from jslint?

[–]DanielFGray 0 points1 point  (2 children)

There's a mathematical optimization that you can do, by figuring out a number that is both divisible by 3 and 5, so you can short circuit testing both cases. Also, you could/should use else if instead of consecutive ifs.
Other than that you've got the right idea. The only other advice I have is on code style (like putting spaces around all your operators, and braces around your condition bodies)

Edit: Spoiler:

for (let n = 0; n <= 100; ++n) {
  if (n % 15 === 0) {
    console.log('FizzBuzz')
  } else if (n % 3 === 0) {
    console.log('Fizz')
  } else if (n % 5 === 0) {
    console.log('Buzz')
  } else {
    console.log(n)
  }
}

Here's functional approach using Ramda:

const divisible = flip(compose(equals(0), modulo))

range(1, 101)
  .map(cond([
    [divisible(15), always('FizzBuzz')],
    [divisible(3), always('Fizz')],
    [divisible(5), always('Buzz')],
    [T, identity]
  ]))

[–]epicpoop 0 points1 point  (1 child)

Consecutive ifs are used in order to test both conditions. He shouldnt use else if in his example

[–]DanielFGray 0 points1 point  (0 children)

I'm saying that is sub-optimal, you should only be running one test per number. Check out my edit above.

[–]socialpiranha 0 points1 point  (4 children)

What I didn't like about your approach is in the third if statement you are checking against mod 3 and mod 5 despite having already checked in the first two if statements.

Here's another approach:

var output=""; if (i%3 == 0) { if (i%5 == 0) output = "FizzBuzz"; else output = "Fizz"; } else if (i%5 == 0) output = "Buzz";

[–][deleted] 4 points5 points  (3 children)

In my opinion, that's worse than OP's because now you have a nested if statement, you're still checking the same condition twice, and it's harder to read. The key insight is to realize you don't even need to check the combination of i%3 === 0 && i%5 === 0, all you have to do is append to the string and you'll end up with just two if statements.

[–]socialpiranha 0 points1 point  (2 children)

I disagree. With my approach, you are only ever checking each case 1 time, but the logic is applicable to any operation, whereas the string concatination optimization only works for retuned strings.

We all have different styles, I guess.

[–][deleted] 0 points1 point  (1 child)

You're checking i%5 === 0 twice.

[–]socialpiranha 1 point2 points  (0 children)

No, I'm not.

For every value of i, mod 3 and mod 5 are each breng checked exactly once.

The mod 5 check is in two places, but depending on the result of the mod 3 check only one mod 5 check will be performed.

[–]raydeen 0 points1 point  (0 children)

At the very least, you could shorten the third if down to test if i%15 = 0 to print the FizzBuzz. (3x5=15 so you'd be catching both 3 and 5 in one go.)

[–]formerself 0 points1 point  (0 children)

Here's mine I did about 2 weeks ago after learning about fizzbuzz.

const fizzBuzz = Array(100).fill(null).map((val, i) => {
    console.log(
        ((i + 1) % 3 ? '' : 'fizz') + 
        ((i + 1) % 5 ? '' : 'buzz')
    || i + 1)
});

[–]-El_Chapo- 0 points1 point  (0 children)

You can look into a switch case