Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid Passing Null #460

Closed
xjamundx opened this issue Mar 15, 2016 · 5 comments

Comments

@xjamundx
Copy link

commented Mar 15, 2016

Not sure if there is a rule for this yet, but it could probably done in a plugin.

Bad:

if (err) return cb(err, null)

Good:

if (err) return cb(err)

Why is that better?

getName(err, name = 'Default') {
   console.log(name); // might end up as `null`
});

@feross

This comment has been minimized.

Copy link
Member

commented Mar 15, 2016

In other words: functions that get called with a parameter named err should only have one parameter.

@xjamundx

This comment has been minimized.

Copy link
Author

commented Mar 15, 2016

I think people are used to typing cb(null, data) when things go well, so sometimes they do cb(err, null) when things go wrong.

There may be a case where you'd want to do cb(err, data), so I'm not sure you'd want to ban anything except null explicitly.

@dcousens

This comment has been minimized.

Copy link
Member

commented Mar 15, 2016

functions that get called with a parameter named err should only have one parameter.

Unless you're callback is just a pass-through:

f(..., function (err, result) {
  console.log('Called back with ', err, result)

  callback(err, result)
})
@feross

This comment has been minimized.

Copy link
Member

commented Mar 16, 2016

@dcousens Good point. So there's no way to reliably enforce this.

@xjamundx I think it would be a mistake to write code that assumes that the value will be undefined just because there's an err. You should check if there's an err explicitly every time.

@xjamundx

This comment has been minimized.

Copy link
Author

commented Mar 16, 2016

Hmm probably a good point (this came up at work today and we found it was
happening in a few places). Probably we should avoid null in general at
this point, but community isn't there yet :)
On Tue, Mar 15, 2016 at 5:34 PM Feross Aboukhadijeh <
notifications@github.com> wrote:

@dcousens https://github.com/dcousens Good point. So there's no way to
reliably enforce this.

@xjamundx https://github.com/xjamundx I think it would be a mistake to
write code that assumes that the value will be undefined just because
there's an err. You should check if there's an err explicitly every time.


You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
#460 (comment)

@feross feross closed this Mar 16, 2016

@lock lock bot locked as resolved and limited conversation to collaborators May 10, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
3 participants
You can’t perform that action at this time.