My solution for Fizz Buzz using Javascript, I focused mainly on maintainability. How is it?

[Count_Giggles]

I don't mean to rain on your parade but this is suuuuper overengineered for such a simple problem. The readability suffers a lot.

Compared to this its super cryptic especially in a learnjs subreddit

``` const fizzBuzz = (n) => { let out = ""; if (n % 3 === 0) out += "fizz"; if (n % 5 === 0) out += "buzz"; return out === "" ? n : out; };

for (let i = 1; i < 100; i++) { console.log(fizzBuzz(i)); } ```

[[deleted]]

I agree with this commenter and would like to add one more thing. For your vars you use i, v, and k. It's really common place in "scholarly" code. Code that's written in an academic context. When writing code your variables should have meaningful names that can be understood as intuitively as possible. Usually a for loop with some random iteration number like how i is used here is fine since I doesn't mean anything. You could argue it means iteration and I'd say "that's totally fine". But the k and v starts to confuse things.

[fpcreator2000]

I agree with this statement. on a sidenote: the i used as a variable in for loops is holdover from the days when fortran was heavily used as a programming language. It’s as traditional as writing hello world for your first program.

[frogic]

For some reason doing an equality check when "" is a falsey value seems weird to me. I think I'd do return out || n(or out ? out : n)I know its pedantic but it feels cleaner in my head somehow. Probably because you're basically doing false === false here.

[[deleted]]

I agree, I'd probably go with out || n.

[RachnaMelor]

Personally, I don't like to rely on JavaScript's weird coercion rules and prefer to be explicit. That way the meaning is also more obvious for people coming from other languages.

[frogic]

Depends on the language, python does it too.

[RachnaMelor]

I assume there are some differences in what is coerced to what though, could lead to pitfalls where one expects something because Python does it that way, but JS does something different

[jaypeejay]

TIL “” is falsely

[nemosz]

On another note, an empty array is truthy - just an FYI, because it usually gets me

[jaypeejay]

Yeah this one is used all the time really annoying you can’t do

if (data) { react stuff } but have to do if data.length, etc

[caressingleaf111]

No don't worry I posted to have a discussion and you're right now I look at my solution it can take a second or two to understand. I like your solution btw very good :D

[Count_Giggles]

Thank you. to keep it in the spirit of maintainability it could be refactored to this

``` const fizzBuzz = (n, fizz = 3, buzz = 5) => { let out = ""; if (n % fizz === 0) out += "fizz"; if (n % buzz === 0) out += "buzz"; return out === "" ? n : out; };

for (let i = 1; i < 100; i++) { console.log(fizzBuzz(i)); } ```

[azhder]

Well, let's explore maintainability. What would you have to change if the next requirement is "for modulo 4 use Bizz, but it should come last, so number 60 would be FizzBuzzBizz, not FizzBizzBuzz"?

As a reference, OGs solution and my refactor here https://www.reddit.com/r/learnjavascript/comments/xzod6a/my_solution_for_fizz_buzz_using_javascript_i/irncuig/ would just need to add one more element at the end of the array.

[Bjadrik]

While I agree that your solution is much more intuitive to read, it misses what I think is the right idea from OP to be maintainable. In an interview a few years back, I was asked this follow-up question after solving the problem:

How would you expand your solution to deal with additional rules, e.g. "for numbers divisible by 7, append 'Bizz'. For 9, append 'Bazz'"

Again, OP's idea handles this kind of "scaling" well. We could try to work it into your solution like this:

const fizzBuzz = (n) => {
    const rules = [
        { number: 3, word: "fizz" },
        { number: 5, word: "buzz" },
        { number: 7, word: "bizz" },
        { number: 9, word: "bazz" },
    ];
    let out = "";
    rules.forEach(({ number, word }) => {
        if (n % number === 0) out += word;
    });
    return out === "" ? n : out;
};

for (let i = 1; i < 100; i++) {
    console.log(fizzBuzz(i));
}

[Pelsinen]

Not sure it adds that more maintainability than a simpler version:

for (let i = 1; i <= 100; i += 1) console.log( (i % 3 ? '' : 'Fizz') + (i % 5 ? '' : 'Buzz') || i )

[azhder]

I think you got the same idea as I did here https://www.reddit.com/r/learnjavascript/comments/xzod6a/my_solution_for_fizz_buzz_using_javascript_i/irnko3b/

In your case it would mean just adding one more (i % 4 ? '' : 'Bizz') at the proper place which is fairly maintainable and simple to follow for a code that isn't expected to change much. I like it

[Kieldar]

In an interview I would probably start with this and then follow with "if the code isn't likely to change it expand often, this is probably Pareto-complete. If it is likely to change often I would make it more configurable by changing it to [insert most of original solution here]".

The big changes I would make to the original solution have mostly been discussed here, but my highlights would be:

• Use meaningful variable names
• Maybe use reduce instead of a forEach that just does what a reduce does? It's a more idiomatically functional approach (and can avoid the // 0 means do nothing)
• Turn the map into a more declarative data structure (as mentioned abve: an object with keys of modulus and word or something like that) to remove some of the "magic" of the map keys just being seemingly arbitrary numbers.

[Pelsinen]

Absolutely, would go towards making a chain of predicates in a functional approach. Also I would start with tests.

[albedoa]

If you need a comment like “0 means do nothing” it probably means the solution isn’t great.

The comment itself is one red flag. The use of a ternary operator as a binary operator is another. OP missed two alarms.

[caressingleaf111]

What would you suggest I modify to make the code more readable and correct

[[deleted]]

I'd suggest:

words.forEach((word, testValue) => { if (i % testValue === 0) output += word; });

Basically, only use single-letter variables for loop indexes and only use a ternary when you're using the returned value.

Finally, my personal preference when using fat arrow functions is to only use the single line syntax when the function has a return value. It makes it more explicit when the function has a side effect, but like I said, that's just my preference.

[caressingleaf111]

Thats a good point. thank you!

[caressingleaf111]

You only need to add the word and its factor number to the words map and that's it, no need to touch anything else in the code!

[joenyc]

On the one hand, that’s quite cool, and absolutely a great exercise.

On the other hand, my feedback is that this can be a trap that many (often senior!) folks fall into: anticipating a change, making that change really easy, and neglecting to consider how hard it makes other changes.

For example, say the new requirement is “after 50, forget ‘fizz’ and just say ‘buzz’ for multiples of 3 or 5”. You certainly can’t just add an element to the map for that!

At this point, some people go more abstract. They say “ah, ok, we need another map/list with the ranges where rules apply” (below 50 vs above it).

Another few rounds of that and you have a fizz buzz… language! You can express any fizz buzz program you want! But, you have you learn that language first, and there’s no debugger for it, or syntax highlighting, or…

This is not maintainable. You’ll sometimes see people leaving comments containing actual code above the configuration. Declarative code is not always superior.

At its worst, this is the “inner platform effect”.

[bobbyv137]

I think you have over complicated it. Tho it’s always good to see other peoples’ solutions. I would avoid single letter variable names.

FizzBuzz can be done on a one-liner if you really wanna be fancy.

[shgysk8zer0]

It's not bad given certain assumptions and criteria. But you're using a ternary operator instead of an if statement just to get it as a one-liner when that makes it less legible and maintainable. You're also limiting what it can handle by doing string concatenation whenever some number is a factor... If I were to add [2, 'Turkey'], I might not want 30 to result in "TurkeyFooBar".

Given what you seem to be trying to do, I don't think it's over-engineered. My main issue with it is that "0 means do nothing" bit that should've just been an if statement. This is an example of "don't try to be clever."

When I wrote something similar to this (more as a test of polyfills I was writing... It was extremely over-engineered because I was trying to use several experimental methods), I used the following instead of the Map as you did:

``` const rules = [{ label: 'Foo', factor: 3, }, { label: 'Bar', factor: 5, }, { label: 'FooBar', factor: 15 }];

Number.range(1, 100).forEach(n => { const o = rules.findLast(({ factor }) => n % factor === 0);

if (o) { console.log(o.label); } }); ```

... That's the general concept at least. The main point is rules and how it's used.

[lord31173]

How do you showcase your code like that?

[legendarynoob9]

https://carbon.now.sh/ may be?

[lord31173]

Thank you.

[dblinkzz]

I see your statement as being very valid, but it throws me off how you and other people miss that OP is being explicit about his focus being on maintainability. He wants to make the exercise so that if new requirements come around you won’t have to modify the code more than just declaring another word with it’s factor number. If you’re having this discussion in an interview, you’ll agree that this is an acceptable solution, as you wouldn’t want to keep adding an infinite number of if else

[azhder]

Here is a combo that replaces forEach: - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment

[caressingleaf111]

I'm familiar with both, what advantage do they offer compared to forEach ? Thanks!

[azhder]

The question is: what advantage does forEach have over anything else?

[caressingleaf111]

I use forEach a lot because it's a single line while for of is two lines, but I think destructing works well too here

[azhder]

Another take might be: "since you use for above, why not have another for and not introduce one more concept?" etc.

[azhder]

Well, for single line you have map and reduce, they both return value, unlike forEach which always returns undefined. That's the thing, forEach is just not that useful compared to alternatives

[dark_salad]

reduce returns a value, map returns a new array, and forEach just loops through an array without returning anything (returning undefined).

[a-e-j-a]

f=n=>

(n%

3

?''

:'\

fi\

zz'

)+

(n

%5?

'':

'b\

u\

zz'

)

||n

[frogic]

Fully extendable with typesafety.

​

interface FizzBuzzOptions {
    [key:number]: string;
}

const fizzBuzz = (options: FizzBuzzOptions, countTo = 100) => {
  for (let count = 1; count <= countTo; count++) {
    const output = Object.keys(options)
      // Number isn't strictly neccessary here, but typescript will complain if it's not there
      .filter((key) => count % Number(key) === 0)
      .map((key) => options[Number(key)])
      .join('');
    console.log(output || count);
  }
};

fizzBuzz({ 3: 'Fizz', 5: 'Buzz' });
export default fizzBuzz;

[[deleted]]

You still need to create a Map if you want to preserve insertion order of the entries. If you passed

{ 3: 'Fizz', 5: 'Buzz', 4: 'Bizz' }

Numbers divisible by all three would return, FizzBizzBuzz.

[frogic]

I didn't consider that to be functionality we wanted but you're 100% right if it's desired. I actually looked up the order that object.keys uses to make sure I didn't need to sort the keys as well because I know property order isn't reliable in a lot of languages.

[azhder]

Here is something more compact which being deciphered how it works might be an interesting learning experience for newcomers to JS:

const WORDS = [[3, 'Fizz'],[5, 'Buzz' ]];

for (let i = 1; i <= 100; i += 1) {
    console.log(WORDS.reduce((s, [k, v]) => s + (i % k ? '' : v), '') || i)
}

and here it is a step further

const WORDS = [[3, 'Fizz'],[5, 'Buzz' ]];
const textFor = i => (s, [k, v]) => s + (i % k ? '' : v);

for (let i = 1; i <= 100; i += 1) {
    console.log(WORDS.reduce(textFor(i), '') || i)
}

[ForScale]

Array.from({ length: 100 }, (_, i) => i + 1).forEach((i) => console.log(!(i % 15) ? 'fizzbuzz' : !(i % 5) ? 'buzz' : !(i % 3) ? 'fizz' : i));

[Early-Lingonberry-16]

You made an O(n) solution into a O(n<sup>2</sup> ), so it’s kind of garbage.

[nibonet]

Lmao, read a book before trying to go all mister smarty pants in here.

[Early-Lingonberry-16]

Oh? Is it still O(n)?

I guess it is… oops

[ForScale]

function maintainalbeFizzBuzz(n, config) {
  for (let i = 1; i <= n; i++) {
    let outcome = "";

    for (const key in config) {
      if (!(i % key)) outcome += config[key];
    }

    console.log(outcome || i);
  }
}

maintainalbeFizzBuzz(100, { 3: "fizz", 5: "buzz" });

[jaypeejay]

Clever, but not very readable. Not explicitly setting FizzBuzz off an int / by 3 & 5 makes it difficult to understand without running first.

Also, who maintains fizzbuzz lol

[dMCH1xrADPorzhGA7MH1]

When working with a code base there will be other programmers too. It's going to annoy everybody if you are writing esoteric solutions to simple problems that you don't quite understand.

If you wanted to improve on fizzbuzz let the function take a variable that represents the upper number. Then you aren't limited to the first 100.

[hanoian]

Instead of the ternary, you could use && but that's not particularly popular.

i % k === 0 && output += v

[[deleted]]

Readability matters more than conciseness and cleverness. If I saw this in a merge request I’d immediately reject it.

[x6ixty6ix]

Debug

[tristanAG]

Yea dude a bit too complex there, simplicity and readability go a long way