2
\$\begingroup\$

I'm preparing for junior developer interviews and am trying to come up with a more interesting/versatile solution to FizzBuzz than I've done in the past. Do you have any ideas of how I might DRY this up? Is it too difficult to read?

const isMultiple = (num, mod) => {
    return num % mod == 0
}

const fizzBuzz = (range, array) => {
    return [...Array(range)].fill('').map((el,i) => {
    i ++
    for (let element of array) { 
        isMultiple(i,element.number) && (el += element.name) 
    }
        if (el == "") el = i
    return el
  });
}

const objArr = [
  {
    number: 3, 
    name: "Fizz"
  },
  {
    number: 5, 
    name: "Buzz"
  }
]

console.log(fizzBuzz(100,objArr))

\$\endgroup\$

5 Answers 5

5
\$\begingroup\$

Habits and syntax

When you are learning to write code you spend a lot of time chasing bugs. For a small project like fizzBizz there is not much that can go wrong. But as the code size increases the number of bugs do to.

Bad habit result in bugs, the worst type of bugs "Hard to see bugs". They are hard to see because bad habits mean poor syntax does not jump out and say WRONG!

Habits tend to stick so when learning anything new it is best to pick up the good rather than the bad habits.

Bad habits.

Not using semicolons

or using them intermittently. JavaScript requires semicolons, if you don't add them yourself the parser will add them for you. It is called ASI Automatic semicolon insertion and it has some odd ways of resolving where to put the missing semicolon. ASI does not show you where it adds semicolons, so the most common way to find out how no semicolons stuff up your code is by spending hours trying to workout what is wrong with the code.

Good JS uses semicolons, it is as simple as that.

Not using {} to delimit code blocks

eg you have if (el == "") el = i. Yes its not required to put {} around { el = i; } But getting used to seeing blocks without them means its hard to spot when you extend a block and forget to add them.

Strict equality and inequality.

The number of times you may need to use == (Equality) or != (Inequality) is vanishingly small. Always use === (Strict equality) or !== (Strict inequality)

Why? The reasons are long and complicated. Strict equality, inequality, compares type and value, while the standard equality, inequality operators will try to convert (coercion) one side of the operation to the same type as the other side then it will compare the values. The list of how types are coerced is long, and not always obvious.

Indentation

Your indentation is all over the place. Indentation helps you read the code, being lax or inconsistent using indentation make code a 1000 times harder to read.

It does not matter what style of indentation 2 or 4 spaces 1 or 2 tabs. Don`t mix and match, Don't forget to indent when you need to and Don't indent when you shouldn't.

All IDEs will auto indent for you, make sure that this is turned on when writing code. Also learn to use the code formatters (All IDEs have them)

Strict mode

Always run code in strict mode.

Good Style

There are many styles, where you put spaces, how you start code blocks, where you break lines, do you prefer if else, or switch, or ternaries ?, or short circuits. Coders will go blue in the face telling you which is best.

Apart from the bad habits above what style you use does not really matter, what matters is that you be consistent. If you put a space between for ( then do it every time.

Inconsistent style makes code harder to read and harder to spot bugs in.

Code noise

Code noise is anything that is not required (with the exception of points in above bad habits). Less code noise takes less effort to read and maintain.

All coders have a "bugs per line" quota. As you gain experience the greater the number of lines of code you can write without having to fix bugs.

There have been many studies into this, and the reality is, for every programmer, fewer lines of code means fewer bugs.

JavaScript has many syntax shortcuts. Learn them and reduce the noise. It takes time to reduce your bugs per line quota, but there are many ways to reduce the number of lines you write.

Code lines are all lines that are not empty. Comments count as lines.

Rewrite

I am not going to show you a better way to write fizz - buzz (there is a zillion on google). The rewrite will use your logic and structure but without the bad habits, a few name changes, removal of some noise and a sprinkling of my personal style preferences.

"use strict";

const isMultiple = (val, mod) => val % mod === 0;
const RULES = [{mod: 3, name: "Fizz"}, {mod: 5, name: "Buzz"}];

const fizzBuzz = (range, rules) => 
    Array(range).fill("").map((str, val) => {
        for (const {mod, name} of rules) { isMultiple(val + 1, mod) && (str += name) }
        return str ? str : str + (val + 1);
    });

console.log(fizzBuzz(100, RULES).join(", "));

\$\endgroup\$
9
  • \$\begingroup\$ If one really wanted to make RULES immutable then the value could be wrapped in a call to Object.freeze() \$\endgroup\$ Jan 22, 2021 at 17:35
  • 2
    \$\begingroup\$ @SᴀᴍOnᴇᴌᴀ that would require both objects in the array and the array to be frozen, Object.freeze([{mod: 3, name: "Fizz"}, {mod: 5, name: "Buzz"}].map(Object.freeze)); and I did not want to explain that in such a simple bit of code. \$\endgroup\$
    – Blindman67
    Jan 22, 2021 at 17:44
  • 2
    \$\begingroup\$ Some people prefer to read and write code without unnecessary semicolons, and let semicolons be inserted by the parser automatically. Plenty of JS projects have no unnecessary semicolons. Just to give a few examples: vercel/next.js, gatsbyjs/gatsby, lodash/lodash, twbs/bootstrap, vue/vue-router/vue-cli, typicode/husky, reduxjs/redux. Is it your experience, that you are spending hours trying to figure out the problem with your code if you don't write unnecessary semicolons? Would you say projects with no-semicolon style code suffer from this problem in a measurable way? \$\endgroup\$
    – user236549
    Jan 23, 2021 at 15:14
  • 1
    \$\begingroup\$ The examples only prove that it is accepted and common amongst professionals to write in either style. Q1: Your clam isn’t coming from personal experience then? Do you know studies that convinced you that not writing unnecessary semicolons causes said problems in any measurable way? Q2: The question is though, if you know mentioned projects suffer from said problem in any measurable way? I haven’t experienced any of the problems you mentioned. \$\endgroup\$
    – user236549
    Jan 23, 2021 at 18:54
  • 1
    \$\begingroup\$ If you check out the most recent google style guide, it no longer gives a justification for the "Semicolons are required" rule, so I don't think we can rely on the above quoted statement anymore. I like the google style guide by the way. I would always prefer using a linter to enforce whichever style guide a project or organization chooses. What I am questioning here is, whether each style is a matter of opinion, which I believe it is, or if in fact, one style is better than the other, which is what you are claiming. \$\endgroup\$
    – user236549
    Jan 24, 2021 at 15:22
6
\$\begingroup\$

Question

Is it too difficult to read?

I wouldn't say it is very difficult, though the callback function to .map() isn't consistently indented. Some commonly adopted style guides recommend a single space following commas1 2 - e.g. instead of

isMultiple(i,number)

add a space to separate the arguments:

isMultiple(i, number)

Other review points

Equality comparisons

A good habit and recommendation of many style guides is to use strict equality operators (i.e. ===, !==). The problem with loose comparisons is that it has so many weird rules one would need to memorize in order to be confident in its proper usage.

Prefer const over let

In the for loop element is declared with let

for (let element of array) { 
    isMultiple(i,element.number) && (el += element.name) 
}

It can be declared with const because it doesn't need to be re-assigned. Using const when re-assignment isn't needed is a good habit because it can help avoid accidental re-assignment and other bugs.

consider destructuring assignment

Furthermore, object destructuring can simplify the code so only the properties needed are pulled out of each element:

for (const {number, name} of array) { 
    isMultiple(i, number) && (el += name) 
}
\$\endgroup\$
2
\$\begingroup\$

Less lines is not always better.

isMultiple(i,element.number) && (el += element.name)

I've only seen this technique with minified or obfuscated code. Use an if statement as normal instead.

array is not a descriptive name. objArr is coupled with fizzBuzz anyways, I would remove this parameter and use the objArr directly.

Use semicolons to designate end of statements

\$\endgroup\$
2
\$\begingroup\$

I like how you implemented it, it just needs a little polishing.

  • Be consistent with semicolon use, indentation, spacing, etc. (maybe grab a pre-configured linter like prettier or standard)
  • When doing [...Array(range)].fill(''), you don't actually need to destructure the array to apply the .fill() afterward, in this particular case
  • Try to shy away from modifying variables. i.e. instead of i++ either do const number = i + 1 or use i + 1 everywhere.
  • Variable naming could be improved.
  • I see no reason to pass in the fizzes list - this game is generally not configurable.

Here's a rewrite that follows these suggestions:

const isMultiple = (num, mod) => num % mod === 0;

const substitutions = [
  {
    factor: 3,
    replacement: 'Fizz',
  },
  {
    factor: 5,
    replacement: 'Buzz',
  },
];

const fizzBuzz = range => [...Array(range)]
  .map((_, i) => {
    const number = i + 1;
    let result = '';
    for (const { factor, replacement } of substitutions) {
      if (isMultiple(number, factor)) result += replacement;
    }
    return result || number;
  });

console.log(fizzBuzz(100));

I'm going to go ahead and throw in an alternative, less-engineered solution too. As the rules for the game are set, and they're simple rules, I don't think there's any reason to extract them out into a list like that.

The previous solution provides a degree of freedom if additional rules are added in a similar format (i.e. if its a multiple of 7, say baz), but will get in the way if the additional rules are not what you were predicting (i.e. baz is special in that if you say baz, you don't say fizz or buzz too). This is why it's good to avoid over-engineering solutions and keep them simple.

const isMultiple = (num, mod) => num % mod === 0;

const getSubstitution = n => {
  if (isMultiple(n, 3) && isMultiple(n, 5)) return 'FizzBuzz';
  else if (isMultiple(n, 3)) return 'Fizz';
  else if (isMultiple(n, 5)) return 'Buzz';
  else return n;
};

const fizzBuzz = length => [...Array(length)]
  .map((_, i) => getSubstitution(i + 1));

console.log(fizzBuzz(100))

\$\endgroup\$
1
\$\begingroup\$

Concept

Most people will consider this overengineered for actual fizz buzz. If you're going to demonstrate it, it is absolutely key that you preface it with "this is how I would solve the fizz buzz type problem in general". You don't want to give an interviewer the impression that you overengineer solutions to simple problems.

Code

You appear to be applying functional patterns, but only halfheartedly. Embracing these patterns can bypass the need for building up el entirely. Currying can also simplify usage of functions like map() and filter().

const range = start => end =>
    [...Array(end-start+1)].map((_,i)=>i+start)

const divides = num => fizz =>
    num % fizz.number === 0

const fizzBuzzAt = fizzes => index =>
    fizzes.
    filter(divides(index)).
    map(fizz=>fizz.name).join('') || index;

const fizzBuzz = fizzes => n =>
    range(1)(n).map(fizzBuzzAt(fizzes))

const fizzes = [
  {number: 3, name: "Fizz"},
  {number: 5, name: "Buzz"},
  {number: 2, name: "Weee"}
]

console.log(fizzBuzz(fizzes)(100))
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.