Javascript Simplify an If-statement which returns a boolean-CodePudding

I want to simplify the code below:

const checkClassValidity = (profName, studentList) => {
  if (
    (profName == "Alexander" || profName == "Elena") &&
    studentList.length > 10
  ) {
    console.log("Too many students!");
    return false;
  } else if (profName == "Eddy" && studentList.length < 20) {
    console.log("Too few students!");
    return false;
  }
  return true;
};

As both if statements execute the same two actions, I want to simplify them like this (they are grammatically wrong):

const checkClassValidity = (profName, studentList) => {
  {
    let msg;
    if (
      (profName == "Alexander" || profName == "Elena") &&
      studentList.length > 10
    ) {
      msg = "Too many students!";
    } else if (profName == "Eddy" && studentList.length < 20) {
      msg = "Too few students!";
    }
    console.log(msg);
    return false;
  }
  return true;
};

I can't find a neat way to do this. Is this a bad idea in the first place and would it be better to leave it as it is?

CodePudding user response：

I would remove the else statement, because on return the function ends.

const checkClassValidity = (profName, studentList) => {
    if ((profName === 'Alexander' || profName === 'Elena') && studentList.length > 10) {
        console.log('Too many students!');
        return false;
    }
    if (profName === 'Eddy' && studentList.length < 20) {
        console.log('Too few students!');
        return false;
    }
    return true;
}

CodePudding user response：

In this case, you can simplify the code by moving the common logic of printing the message and returning false outside of the if statements. You can do this by using a separate variable to store the message and a single if statement to check if the message is set.

Here is an example of how you can simplify the code:

const checkClassValidity = (profName, studentList) => {
  let msg;
  if (
    (profName == "Alexander" || profName == "Elena") &&
    studentList.length > 10
  ) {
    msg = "Too many students!";
  } else if (profName == "Eddy" && studentList.length < 20) {
    msg = "Too few students!";
  }

  if (msg) {
    console.log(msg);
    return false;
  }

  return true;
};

CodePudding user response：

I believe you are trying to do something like this. I simply set another variable for isValid and by default it is false. If it reaches the else block then we know that it must be true at this point. Before we exit the function we log out the message and return our new isValid boolean. Now you have cleaner code with less duplication.

I would suggest creating a class for your teacher names and do further refactoring but that's a topic for another question.

    const checkClassValidity = (profName, studentList) => {
        let msg = '';
        let isValid = false;

        if ((profName == 'Alexander' || profName == 'Elena') && studentList.length > 10)) {
            msg = 'Too many students!';
        } else if (profName == 'Eddy' && studentList.length < 20) {
            msg = 'Too few students!';
        } else {
            isValid = true;
        }

        console.log(msg);
        return isValid ;
    }

CodePudding user response：

If you want it more readable, go for this:

const checkClassValidity = (profName, studentList) => {
  let valid = true;
  switch (profName) {
    case 'Alexander':
      studentList.length > 10 ? valid = false : '';
      break;
    case 'Elena':
      studentList.length > 10 ? valid = false : '';
      break;
    case 'Eddy':
      studentList.length < 20 ? valid = false: '';
      console.log('Too few students!');
      break;
    default:
      return valid;
  }
}