2
\$\begingroup\$

I spent some time today trying to write FizzBuzz in a functional/declarative style. I thought it would be a good chance to get some feedback on it since I've been doing functional programming for about a year now.

(I know there are some similar questions asked here before, but their approach to the code has been different to mine and it would be good to get feedback more specific to my code)

The main things I want to know are:

  • How consistent is this code with the best practices in functional and declarative programming?
  • Are there any bad practices in it that I should be aware of?
const isFizz = number => number % 5 === 0
const isBuzz = number => number % 3 === 0

const newArrayInRange = (min, max) => [...Array(max + 1).keys()].slice(min)

const fizzBuzz = (min, max) => newArrayInRange(min, max)
  .map(number => {
    if (isFizz(number) && isBuzz(number)) {
      return 'fizzbuzz'
    }
    if (isFizz(number)) {
      return 'fizz'
    }
    if (isBuzz(number)) {
      return 'buzz'
    }
    return number
  })
  .join('\n')

console.log(fizzBuzz(1, 100))
\$\endgroup\$
0

3 Answers 3

3
\$\begingroup\$

Maybe some changes

  • Removing the arrays needed to hold the counter, its a huge overhead just for a counter.
  • Avoiding excessive logic statements using (num % val) to return strings and using the empty string to add a number (see code marked /*A*/)
  • Putting everything into an array to join when done as you have done. It is the quickest way to build a long string in JavaScript.

Thus we end up with the following...

const fizz = num => num % 5 ? "" : "fizz";
const buzz = num => num % 3 ? "" : "buzz";
const fizzBuzz = num => fizz(num) + buzz(num);
const fizzBuzzer = (min, max) => {
    const res = [];
    do {
        const fb =  fizzBuzz(min);
        res.push(fb ? fb : min);       /*A*/
    } while (min++ < max);

    return res.join("\n");
}

Personally the function fizz, buzz, fizzBuzz are just adding code without good reason and would need to be closed over to avoid polluting what ever scope it is in.

Thus the 3 function become the expression right of const fb =

Also I am not a fan of arrow functions in an open scope so using a function declaration to ensure accessibility.

function fizzBuzzer(min, max) {
    const res = [];
    do {
        const fb =  (min % 5 ? "" : "fizz") + (min % 3 ? "" : "buzz");
        res.push(fb ? fb : min);
    } while (min++ < max);

    return res.join("\n");
}

You ask

How consistent is this code with the best practices in functional and declarative programming?

Best practice, well that is subjective, contextual and would only have comparative meaning when compared to "bad" code?

Your code is not bad, it works , it would have been worse 3 years back due to the way JS engines handled arrays, but now most optimizers recognize the pattern and make it fly.

There is a zillion ways to write any bit of code. JS programmers can still not agree on the use of semicolons and whether or not automatic semicolon insertion was a good idea or not, so how is any bit of code ever going to be best practice.

Are there any bad practices in it that I should be aware of?

OMDG Yes there is.... Semicolons, where are they?


// Example of bad
const fizzBuzzard = (min, max) => {
    const next = n => n <= max ? order[n % 15] : () => "";
    const txt = w => n => (w ? w : n + "\n") + next(++n)(n);
    const [n, b, f, fb] = [txt(), txt("buzzard\n"), txt("fizz\n"), txt("fizzBuzzard\n")];
    const order = [fb, n, n, b, n, f, b, n, n, b, f, n, b, n, n];
    return next(min)(min);
}
console.log(fizzBuzzard(1, 100));

\$\endgroup\$
2
  • \$\begingroup\$ Thanks for your reply. I'm just a bit unclear about the example. My understanding was that using while loops and min++ is not typical of functional programming, so your example is more of an imperitive style. Is that correct? And is this just done for performance reasons, or also readability? Good point about the semi-colons. I tend to lean to the opinion that it's OK to rely on minifiers and automatic semicolon insertion, but I know some developers (including Kyle Simpson) believe it's better to use them. I'll have to do some more research and reconsider my views about this. \$\endgroup\$
    – Andre O
    Commented Jul 27, 2019 at 4:38
  • \$\begingroup\$ @AndreO JavaScript is not a functional language. All functions have side effects because they use a shared heap and a finite call stack. This means that functional loops (via recursion) can not be pure (call stack overflow can occur at any iteration) and loops via Array callback methods just move the syntax for the loop to the engine they are thus declarative loops. Functional Javascript as a paradigm is about style of which its most important part is the concept of abstract level side effects. The example I gave (first) is functional within this requirement. \$\endgroup\$
    – Blindman67
    Commented Jul 27, 2019 at 23:24
2
\$\begingroup\$

How consistent is this code with the best practices in functional and declarative programming?

I dunno. Unless you're doing something fancy like hand-rolling monads, you might just be "programming with functions" and not really doing "functional programming." You could ask Crockford about that, if he manages not to get himself dis-invited from your next local tech conference.

Are there any bad practices in it that I should be aware of?

Looks alright to me.


Personally I like the other answer here. I voted for it and hope it wins the Checkmark Election, because it's very practical.

But it's not what I'd call "functional."

const fizzbuzz = (min, max) =>
    Array(max + 1).fill('', min)
        .map((v, i) => i % 5 ? v : 'fizz')
        .map((v, i) => i % 3 ? v : v + 'buzz')
        .map((v, i) => v || i)
        .join('\n').trim()

If the code in your question is "functional," I suppose this is "more functional," because we got rid of more of the flow control, chained even more functions together, and threw even more lambdas in there. I definitely wouldn't go as far as calling it "declarative," though. It still looks imperative to me; a series of commands to be executed in a particular order. So, I see no reason to value something like this over Blindman67's approach.

Some tasks are better suited to a declarative style (data transformations, things where it doesn't really matter exactly what order things happen in; think XSLT for example) and some are better suited to an imperative style (anything where you need precise flow control). Fizzbuzz could surely be done in a declarative way, with the right language or libraries. But with plain JavaScript, it seems much more straightforward to do it in an imperative way.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks, you make some really good points about the limitations of doing functional programming with pure JavaScript, and also the types of tasks which functional/declarative code is/isn't suited to \$\endgroup\$
    – Andre O
    Commented Jul 27, 2019 at 5:01
-1
\$\begingroup\$

First I would like to say that code can be functional to a degree, so I will point out what I like and what could be (even) more functional.

I like isFizz and isBuzz because they are declarative and pure functions (i.e. they have no obvious side effects and are deterministic). I also like that the map does not use intermediate variables, because that would be a more imperative approach.

A suggestion would be trying to reduce the potential number of evaluations in the map (for each number, both isFizz and isBuzz are potentially executed twice) without using intermediate variables. My suggestion can be found below in the toTerms function or the Ramdba pseudo code.

After taking the course Functional-Light JavaScript v3, I have attempted to write a declarative/functional FizzBuzz implementation too.

In this course, Kyle Simpson recommends gradually converting to declarative style, which I found helpful. Also it resulted in a way over-engineered implementation, but I think it does show parts of the code that I really had to think about before finding an appropriate declarative alternative.

The implementation is in this fiddle, with my intermediate progress too. It also contains suggestions for further refactoring. This is the state at time of writing:

const range = (function rangeInner(acc) {
	return (first, last) => {
    if(first > last) {
      return acc;
    }
    return rangeInner([...acc, first])(first + 1, last);  	
  }
})([]);

const isMod3 = n => n % 3 === 0;
const isMod5 = n => n % 5 === 0;

const predicatesAndTerms = [
    [isMod3, "Fizz"],
    [isMod5, "Buzz"]
]

const toTerms = n => predicatesAndTerms
	.reduce((acc, [predicateFn, term]) => acc += predicateFn(n) ? term : "", "");

const toFizzBuzz = n => toTerms(n) || n.toString();

console.log(range(1, 15).map(toFizzBuzz));

In an attempt to avoid potentially evaluating isMod3 (isFizz in the question) and isMod5 twice for each number, I refactored to a reduce over an array of "predicate" and "term" tuples, that is evaluated by a ternary operator that returns a value directly. I also tried to avoid intermediate variables as much as possible, forcing myself to create tiny cohesive functions instead.

In an earlier version, I had a similar implementation of range as in the question, but I refactored it to try out recursion combined with a closure for the acc parameter, with the option to refactor it using R.curry later, (where R refers to Ramda).

Pseudo-code for further refactoring toFizzBuzz with Ramda could be:

const orDefault = fn => n => fn(n) || n; // looks like R.defaultTo, but executes fn over n first.
const toTermsOrDefault = orDefault(toTerms);
const toString = n => n.toString();
const toFizzBuzz = compose(toString, toTermsOrDefault);

Good luck learning FP JavaScript! If you want to discuss FP JS, I'm always interested.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Welcome to Code Review! You have presented an alternative solution, but haven't reviewed the code. Please explain your reasoning (how your solution works and why it is better than the original) so that the author and other readers can learn from your thought process. If you want to get your own declarative code reviewed, please ask a new question. Thanks :) \$\endgroup\$
    – Zeta
    Commented May 24, 2020 at 10:38
  • \$\begingroup\$ Thank you pointing that out, I have updated my original answer. \$\endgroup\$
    – mdworld
    Commented Jun 8, 2020 at 10:01
  • \$\begingroup\$ I'd switch both parts. The original user is interested in a review first, an alternative solution afterwards. Other than that this seems like an overall better answer :) \$\endgroup\$
    – Zeta
    Commented Jun 8, 2020 at 11:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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