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.