0
\$\begingroup\$

What are the pros and cons of each option, considering long-term implications (increasing the number of functions / parameters, other developers taking over, etc.)?

Option 1: removes the need to pass foo and bar to each method, but will create nested functions that are hard to follow.

const myFunction = ({foo, bar}) => {
  const results = []

  const function1 = () => {
    return foo + bar;
  }

  const function2 = () => {
    return foo * bar;
  }

  const res1 = function1();
  const res2 = function2();

  results.push(res1, res2);
}

Option 2: you pass the parameters to each function, but remove the nesting, which in my opinion makes it more readable.

const function1 = ({foo, bar}) => {
  return foo + bar;
}

const function2 = ({foo, bar}) => {
  return foo * bar;
}

const myFunction = ({foo, bar}) => {
  const results = []

  const res1 = function1({foo, bar});
  const res2 = function2({foo, bar});

  results.push(res1, res2);
}

I would prefer to know how to improve my functional approaches here. Thank you!

\$\endgroup\$
2
  • \$\begingroup\$ I'm afraid this question does not match what this site is about. Code Review is about improving existing, working code. The example code that you have posted is not reviewable in this form because it leaves us guessing at your intentions. Unlike Stack Overflow, Code Review needs to look at concrete code in a real context. Please see Why is hypothetical example code off-topic for CR? \$\endgroup\$ Commented Nov 29, 2020 at 10:50
  • \$\begingroup\$ Thank you, @Vogel612. \$\endgroup\$ Commented Nov 29, 2020 at 10:59

1 Answer 1

2
\$\begingroup\$

Option 1

The nested function is not bad (depending on the situation). Of course, they bring more complexity and reduces extensibility. But in some cases, you wish to bundle some function that works with the main function output (well, it's not very different from the class).

Of course you should return object instead of the list for the usage convenience:

const myFunction = ({foo, bar}) => {
  return {
    function1() {
        return foo + bar
    },
    function2() {
        return foo * bar;
    }
  }
}
myFunction().function1()

To sum up, this design can simplify usage in some cases. However, the worst thing about this pattern is that these functions are not extendible and you can't use partial functionality (like using only function1). In most cases, I recommend avoiding this unless you are writing a library/helper function and which to simplify usage by attaching other chain function to the main output (fetch is a good example).

Option 2

Regarding the second option, it works fine as long as your function is not too complex and you don't have too many parameters. You can partially solve the issue with a lot of parameters using TypeScript (because you can statically describe them).

However, if possible I recommend changing logic a bit. Instead of requiring all parameters why don't you require the output of functions? Then you can have multiple functions which return different output + you can also write your own implementation for one part. This option opens a way for unlimited possibilities.

Example:

const myFunction = ({foo, bar}) => {
    return [
        foo,
        bar
    ]
}

myFunction({
    foo: function1(),
    bar: function1(),
})

// You can "extend" this function:
myFunction({
    foo: function1(),
    bar: myCustomFunction(),
})
\$\endgroup\$
1
  • \$\begingroup\$ Interesting. The problem wouldn't really be extensibility, these are just functions like "getUsersA", "getUsersB", etc. that just need to be encapsulated. I'm afraid that later on it will require even more parameters, so multiple changes would have to be done. Functions will most likely not be required to be ran individually. All require parameters like "limit=". But I couldn't provide a concrete example so I moved the question to Stackoverflow. \$\endgroup\$ Commented Nov 29, 2020 at 11:04

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.