Nodebestpractices: Using named parameters in functions

[AbdelrahmanHafez]

Inspired by this awesome article
I've found it super useful to use named parameters when using functions instead of unnamed parameters.

The benefits are:
1- Order of parameters doesn't matter.
2- Default parameters.
3- Anyone who reads the code knows what this parameter is instantly.

// use named parameters
const add = ({ x, y }) => x + y;

let sum = add({ x: 4, y: 5 });


// avoid
const add = (x, y) => x + y;

let sum = add(4, 5);

[goldbergyoni]

@AbdelrahmanHafez Welcome! Interesting idea for a best practice.

I don't know if it isn't too subjective as it pollutes the code with overhead that theoretically could be augmented by the IDE (see below), maybe advice a clear strategy on parameter names + optionally use named parameters?

I'm not sure if this isn't the IDE/tooling job to show the param name for the caller of the function. For example, webstorm prepend the param name to the param value, see prtscn here:
https://www.jetbrains.com/help/phpstorm/viewing-method-parameter-information.html

[AbdelrahmanHafez]

How we name our parameters isn't the point here, we should have a section on naming variables in general if we don't already have one, but that's not what this is about.

I haven't done enough research on how IDEs show the parameter names, however, I use VSCode, and it is not consistent as it should be, specially if you require your functions using

require(path.resolve('./path/from/root'));

Also, it's not just about writing the code, it helps more in the process of reading the code and understanding it. Take the 2 following functions for example:

findUsersByRole('admin', true, false);

findUsersByRole({ role: 'admin', withContactInfo: true, includeInactive: false });

Which one reads better?

[goldbergyoni]

The second. But the same value can be provided by some IDEs (see the image, it looks exactly like your second example without changing anything in the code...), other IDEs will follow soon. Making all app functions receive object is not a very trivial and highly-standard. Consequently, I find it to be a good idea but not sure if compelling enough to be a "best practice"

[AbdelrahmanHafez]

VSCode can't get some functions if they're hooked in the global object somewhere in the project, in order to get these functions parameters, it'll have to loop through all the project's files. There are functions where it simply can't know what this function is on the time of writing/reading, these functions are assigned somewhat dynamically.

I agree though, it may not be a "best" practice, however, let's leave the issue open for a while and see what others think.

[goldbergyoni]

Sounds like a good idea, you may link to this issue from some groups or ask at reddit + we can also create a Twitter Poll

[AbdelrahmanHafez]

Sure, a Twitter poll sounds like a good idea.
I'll post on Reddit as well.

[goldbergyoni]

@AbdelrahmanHafez https://twitter.com/nodepractices/status/1012413507667718146

[lhorie]

Not a good addition IMHO. Here are a few (of numerous) examples that get awkward:

getConfig({config: 'foo.bar.baz'}) // yo dawg I heard you like config
// vs
getConfig('foo.bar.baz')

const fnWithOptionalArgs = ({arg = 'default', another = 'default'} = {arg: 'default', another: 'default'}) => {}
// vs
const fnWithOptionalArgs = (arg = 'default', another = 'default') => {}

const variadicArgs = ({args = []} = {args: []}) => {}
// vs
const variadicArgs = (...args) => {}

const withName = ({item}) => item.name
users.map(user => withName({item: user}))
// vs
const withName = item => item.name
users.map(withName)

The "best practice" is to use named params when they make sense. But the whole point of a best practice is that you can generally follow a best practice blindly without understanding the reasoning and intricacies behind it, but even then there's a high chance you're doing the right thing.

Validating named parameters correctly takes discipline and if done poorly can lead to more bugs than if you just used inline params.

[AbdelrahmanHafez]

I agree, it's not a good pattern when a function takes only one parameter and returns only one thing, so there's no point in naming the parameters, like the getConfig, and withName example.

However, for the other examples, you don't need to repeat the parameters again, you can simply set the default to an empty object like so:

const fnWithOptionalArgs = ({arg = 'default', another = 'default'} = {}) => {}
// vs
const fnWithOptionalArgs = (arg = 'default', another = 'default') => {}

There's probably a typo in the variadicArgs example, I am not sure what you're trying to achieve with such a function, but I assume it can be done with the following as well.

const variadicArgs = ({...args}={}) => {}
// vs
const variadicArgs = (...args) => {}

// a use case where I have actually used it yesterday was
const doStuffToOrder = async ({ _id, ...query }, { status, otherField }) => {
  // do this to order anyway
  await Order.updateOne({ _id }, { status });

  // update order only if it fits the rest of the criteria
  await Order.updateOne({ _id, ...query }, { otherField });
};

EDIT: My variadicArgs doesn't do exactly the same thing, since this returns an object with their parameter names instead of an array of values, which can of course be converted to an array

// if the order of the elements doesn't matter
Object.values(args);

// if the order is important
const variadicArgs = ({ args = [] } = { }) => { };

[lhorie]

you don't need to repeat the parameters again

Eh, depends. If you're using Flow.js, for example, defaulting to an empty object will result in a type error.

Also, that pattern isn't foolproof, e.g.

function buildThing({constructor = 'Bob'} = {}) {
  console.log(constructor + ' built it');
};

buildThing(); // function Object() { [native code] } built it

There's probably a typo in the variadicArgs example, I am not sure what you're trying to achieve with such a function

I fixed the typo, thanks for catching it.

Here's a real world variadic signature example (from React/any vdom lib):

// definition
React.createElement = h = (tag, props, ...children) => ...

// usage
h('div', {title: 'hello'}, 'hello', 'world')

// with RORO
h({tag: 'div', props: {title: 'hello'}, children: ['hello', 'world']})

In addition to reducing readability, the RORO version would affect performance on several levels (bundle size, hidden class deoptimizations, GC pressure) - and yes, perf kinda matters when we talk about vdoms.

My point is mainly that if I can keep poking holes in the "best practice", then there's a good chance it isn't a generically safe enough practice to qualify as a best practice.

[AbdelrahmanHafez]

You got a point there, it's not a best practice that should be followed blindly without knowing the possible downsides.