I'm writing a function to validate for input arguments. But I'm not sure it's the best practice to use.

I have a function that validates like this,

const isValidStudyId = (id) => {
  if (validator.isUUID(id) === false || isValidArguments([id]) !== true) {
    console.log(`${id} is invalid Study Id`);
    return { valid: false, message: `${id} is invalid id!` };
  }
  return true;
}

And use this like this,

  const isValidId = utils.isValidStudyId(studyId)
  if (isValidId !== true) {
    res.status(400).send({
      message: isValidId.message
    });
    return;
  }

And I don't like the way it check validity like

if (isValidId !== true)

this way. Because it's returning sometimes boolean, sometimes an object. Could you suggest other ways to do? I'm not very familiar with the javascript, and I wanna learn more about what is the best practice in this sort of situation.

CodePudding user response：

The Normal Way

If you're referring solely to what structure you should use, perhaps returning something like {valid: true} and check the valid property, as the message property is only checked when valid is false, it's not necessary to set one when valid is true, unless you want to.

The Weird (and maybe ever so slightly nonstandard) Way

Another option would be to use something like callback functions to structure your flow differently.

// 'next' is a function that takes two arguments (error, message)
const checkStudyID = (id, next) => {
    // '=== false' is somewhat redundant if these checks always return booleans
    if(!validator.isUUID(id) || !isValidArguments([id])) {
        console.log(`${id} is an invalid Study ID`);
        return next(true, `${id} is invalid id!`);
    }
    next(false);
}

// --- USAGE ---
utils.checkStudyId(studyId, (err, msg) => {
    if (err) return res.status(400).send({message: msg});
});

CodePudding user response：

Change it to validateStudyId instead that returns a message why it's invalid if it's invalid, otherwise returns a falsy value.

const validateStudyId = (id) => {
  if (validator.isUUID(id) === false || isValidArguments([id]) !== true) {
    const message = `${id} is invalid Study Id`;
    console.log(message);
    return {
      valid: false,
      message
    };
  } // else undefined
}

and then you can just test it like it's boolean and even chain several such checks together in a way that stops at the first truthy response, like this:

const o = validateStudyId(id) || validateOtherThing() || validateThirdThing();
if (o) {
  res.status(400).send(o); // no need to create a new object
}