I came across pieces of code in the app codebase I work on quite a few times which seem wonky, to say the least. In an event that there’s an error, a callback should return the error object and only the error object.

The code, in question, I’m talking about goes something like this -

if (error || !user) {
    return callback(error);  
}

Now, in this scenario, if the user object turns up null for some reason, you are essentially returning a callback containing a blank error object which serves no purpose.

What should, ideally be returned is a 404 if no user is found which is a separate if condition in itself. The error check should be, well, that only - a check for errors.

My first example uses Vanilla JS with the second one using Lo-dash, which makes for cleaner code.

Example 1 (using Vanilla JS):

if (error) { return callback(error); }
if (user === null) { return callback(new Error('User not found!')); }  

if (error) { return callback(error); }
if (!user) { return callback(new Error('User not found!')); }

Example 2 (using Lo-dash):

if (error) { return callback(error); }
if (_.isNull(user)) { return callback(new Error('User not found!')); }