4
\$\begingroup\$

I tried to code the fizzbuzz program following the functional programming rules in javascript. Please check my code and give feedback. thank you.

const isDivisible = (number, fizz) => number % fizz === 0;
const getSubstitutes = (
  number,
  substitutes,
  { currentIndex = 0, result = "" } = {}
) => {
  if (currentIndex === substitutes.length) return result;
  const { divisor, substitute } = substitutes[currentIndex];
  const newResult = result + (isDivisible(number, divisor) ? substitute : "");
  return getSubstitutes(number, substitutes, {
        currentIndex: currentIndex + 1,
        result: newResult,
  });
};
const fizzBuzz = (number, substitutes, currentIndex = 1) => {
  if (number < currentIndex) return;
  console.log(getSubstitutes(currentIndex, substitutes) || currentIndex);
  fizzBuzz(number, substitutes, ++currentIndex);
};
const substitutes = [
  {
        divisor: 3,
        substitute: "fizz",
  },
  {
        divisor: 5,
        substitute: "buzz",
  },
];
fizzBuzz(100, substitutes);
\$\endgroup\$

3 Answers 3

6
\$\begingroup\$

Overall it looks like you're missing out on most of the benefits of a functional approach and over-complicating your code. First glance it's basically impossible to work out what it's all doing without following each variable through everything.

Per the previous answer you don't seem familiar with common functional techniques like using .map, pipelines, or avoiding side-effects

Below is less a review of your code but a different solution that highlights the techniques you're missing so you can see them in action

const fizzOrBuzz = (a, b, aPhrase, bPhrase) => (
    (n) => (
        (n % (a*b) === 0) ? aPhrase + bPhrase :
        (n % a === 0) ? aPhrase :
        (n % b === 0) ? bPhrase :
        n
    )
)

const generateNLengthArray = (n) => [ ...Array(n).keys()].map(i => i + 1)

const mapArrayTo = (f) => (array) => array.map(i => f(i)) 

const joinArray = (array) => array.join('\n')

const pipe = (...fns) => (x) => fns.reduce((v, f) => f(v), x) // One-liner pipeline pattern

const fizzbuzz = pipe(
    generateNLengthArray,
    mapArrayTo(fizzOrBuzz(3, 5, 'fizz', 'buzz')),
    joinArray
)

console.log(fizzbuzz(100))

One of the benefits of functional programming is breaking everything down into discrete, easily understandable functions, and then composing them together so it's crystal clear what's happening.

In this case defining fizzbuzz as a pipeline of three separate functions makes it clear that you're 1. creating an array of a specific length. 2. mapping the values of that array to either fizz or buzz and 3. joining the results together

\$\endgroup\$
5
\$\begingroup\$

Suggestions from previous answers are great and I have nothing to add except for giving advice to not overuse functional programming in Javascript. Here is my solution:

const range = (max) => [...Array(max).keys()].map((i) => i + 1);
const fizzbuzz = (x) => {
    if (x % 3 === 0 && x % 5 === 0) return 'FizzBuzz';
    if (x % 3 === 0) return 'Fizz';
    if (x % 5 === 0) return 'Buzz';
    return x.toString();
};
const result = range(100).map(fizzbuzz);
\$\endgroup\$
1
  • 1
    \$\begingroup\$ definitely the most usable answer in practice \$\endgroup\$
    – Coupcoup
    Nov 20, 2020 at 19:46
4
\$\begingroup\$

Parameter name (number, fizz) => number % fizz === 0 The fizz isn't very indicative of what it represents. Consider divisor or possibleDivisor instead.

Avoid reassignment in functional programming: your ++currentIndex reassigns that variable. If you needed recursion, pass the variable plus one instead of incrementing the variable and passing it: use currentIndex + 1.

Move required side-effects to the edges of the script, since you want a functional approach - make the fizzBuzz function functional by having it return a string which can be logged by the caller, instead of fizzBuzz itself log the results. (console.log is a side-effect, so including it in a function makes it non-functional)

getSubstitutes recursion? The recursive approach used in getSubstitutes, while it works, seems a bit off to me. Functional programming excels at pure transformation of data (and in JavaScript, with object/array methods), and given the substitutes array, I think a .filter / .map would be a bit more appropriate, without any recursion.

fizzBuzz recursion too? - if you feel like it, you can create an array of the values to be logged all at once by using Array.from instead of the somewhat imperative approach of incrementing and testing currentIndex:

const isDivisible = (number, divisor) => number % divisor === 0;
const getSubstitutes = (number, substitutes) => substitutes
  .filter(({ divisor }) => isDivisible(number, divisor))
  .map(({ substitute }) => substitute)
  .join('');
const fizzBuzz = (length, substitutes) => (
  Array.from(
    { length },
    (_, i) => getSubstitutes(i + 1, substitutes) || i + 1
  )
    .join('\n')
);


const substitutes = [
  {
        divisor: 3,
        substitute: "fizz",
  },
  {
        divisor: 5,
        substitute: "buzz",
  },
];
console.log(fizzBuzz(100, substitutes));

Recursion is useful in functional programming when you need persistent state (like with a game, or memoization), but it can make the logic somewhat difficult to understand at a glance - I'd only use it when other approaches don't work well, or when the recursive logic to implement is natural (like with a factorial).

\$\endgroup\$
3
  • \$\begingroup\$ Thank you very much for your answer. I want to is that in your code const getSubstitutes = (number, substitutes) => substitutes .filter(({ divisor }) => isDivisible(number, divisor)) .map(({ substitute }) => substitute) .join(''); is the function inside filter a pure function? If not should I try to make it a pure function? and what would be a good approach to do it? Thank you \$\endgroup\$ Nov 20, 2020 at 0:36
  • \$\begingroup\$ Yes, everything there is pure - it causes no side-effects and depends only on the values of the parameters, producing the same result every time given the same input. \$\endgroup\$ Nov 20, 2020 at 1:14
  • \$\begingroup\$ But substitutes.filter(({ divisor }) => isDivisible(number, divisor)) takes number variable from outside scope. ({ divisor }) => isDivisible(number, divisor) in this function number is not a parameter. \$\endgroup\$ Nov 20, 2020 at 6:25

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.