admin管理员组

文章数量:1405393

I am using ExpressJS and needed to run and pass ESLint rules on my code. There is a rule "consistent-return", which is thrown for code like this:

function getUsers( req, res, next ){
  if( req.userIds.length === 0 ){
    return next();
  }

  collection.find({"_id": {$in: req.userIds}}, function( err, doc ){
    if( err ){
      return next();
    }

    req.users = doc;
    return next();
  });//find()
}//getUsers()

Now, this code gives 'consistent-return' error. There seem to be 2 possible changes, which remove this error, I'm not sure which of these is correct.

function getUsers( req, res, next ){
  if( req.userIds.length === 0 ){
    return next();
  }

  collection.find({"_id": {$in: req.userIds}}, function( err, doc ){
    if( err ){
      return next();
    }

    req.users = doc;
    return next();
  });//find()

  return false; // adding return false passes the ESLint error
}//getUsers()

OR

 function getUsers( req, res, next ){
  if( req.userIds.length === 0 ){
    return next();
  }

  // adding return in below line passes the ESLint error.
  return collection.find({"_id": {$in: req.userIds}}, function( err, doc ){
    if( err ){
      return next();
    }

    req.users = doc;
    return next();
  });//find()
}//getUsers()

Please advise which is the right way to go about it. Thanks.

I am using ExpressJS and needed to run and pass ESLint rules on my code. There is a rule "consistent-return", which is thrown for code like this:

function getUsers( req, res, next ){
  if( req.userIds.length === 0 ){
    return next();
  }

  collection.find({"_id": {$in: req.userIds}}, function( err, doc ){
    if( err ){
      return next();
    }

    req.users = doc;
    return next();
  });//find()
}//getUsers()

Now, this code gives 'consistent-return' error. There seem to be 2 possible changes, which remove this error, I'm not sure which of these is correct.

function getUsers( req, res, next ){
  if( req.userIds.length === 0 ){
    return next();
  }

  collection.find({"_id": {$in: req.userIds}}, function( err, doc ){
    if( err ){
      return next();
    }

    req.users = doc;
    return next();
  });//find()

  return false; // adding return false passes the ESLint error
}//getUsers()

OR

 function getUsers( req, res, next ){
  if( req.userIds.length === 0 ){
    return next();
  }

  // adding return in below line passes the ESLint error.
  return collection.find({"_id": {$in: req.userIds}}, function( err, doc ){
    if( err ){
      return next();
    }

    req.users = doc;
    return next();
  });//find()
}//getUsers()

Please advise which is the right way to go about it. Thanks.

Share Improve this question edited Jan 5, 2017 at 13:05 Mohit Bhardwaj asked Jan 5, 2017 at 12:21 Mohit BhardwajMohit Bhardwaj 10.1k7 gold badges40 silver badges65 bronze badges
Add a ment  | 

2 Answers 2

Reset to default 8

The rule here points to a real consistency problem: sometimes you seem to return a value, and sometimes you don't. And it only works because the returned value is ignored. The only point of your return statement is to branch out of executing the rest of the function.

Your code here is misleading and it hurts the readability.

Replace

if( err ){
  return next();
}

with

if( err ){
  next();
  return;
}

The whole not confusing code:

function getUsers( req, res, next ){
    if( req.userIds.length === 0 ){
        next();
        return
    }
    collection.find({"_id": {$in: req.userIds}}, function( err, doc ){
        if( !err ){
            req.users = doc;
        }
        next();
    });//find()
}//getUsers()

Let us analyze your code a bit, with some details stripped out:

function getUsers( req, res, next ){

  if( req.userIds.length === 0 ){
    return next();
  }

  collection.find({"_id": {$in: req.userIds}}, function( err, doc ){
    // lotsa code here
  });

}

Here, inside the function getUsers(), there can two code-paths: either an error condition (when req.userIds is empty) or not.

Depending on whether the error condition is satisfied, the function getUsers() is either returning something (next()) or nothing (actually returns undefined implicitly). Note that collection.find() is executed in the second case, which itself may return something within it, but getUsers() won't know about it.

Now, as the ESLint docs state, a function can either return something or nothing (implicit undefined), but not both (which is inconsistent behaviour); otherwise a consistent-return error is thrown.

So, to resolve the error, both Denys' code and your own second possible change will work. In Denys' code, both code paths return nothing (implicit undefined) while in yours, they both return something.

本文标签: