I am trying to extract the date of birth from someone's ID number and make sure that the days of specific months are accurate, for example: if February has more than 29 days, it should return false but it is returning true instead. I think I just need a fresh pair of eyes on this code.

function checkDateOfBirth(idNumber) {
    const year = idNumber.substring(0, 2);
    const month = idNumber.substring(2, 4);
    const day = idNumber.substring(4, 6);
    let isDOBValid = false;
    const dateRegex = /^([12]\d{3}-(0[1-9]|1[0-2])-(0[1-9]|[12]\d|3[0-1]))$/;
    const cutOffDate = new Date().getFullYear() - 2000;
    const fullDate =
        (year > cutOffDate ? "19" : "20")   year   "-"   month   "-"   day;

    if (
        day > 31 ||
        month > 12 ||
        (month == 2 && day > 29) ||
        ((month == 4 || month == 6 || month == 9 || month == 11) && day > 30)
    ) {
        isDOBValid;
    } else {
        isDOBValid = true;
    }

    if (dateRegex.test(fullDate) == false) {
        isDOBValid;
    } else if (dateRegex.test(fullDate) == true) {
        isDOBValid = true;
    }
    return isDOBValid;
}

CodePudding user response：

Some Improvements

Let me start off by suggesting a couple simplifications.

First, let's start with your second if statement. The purpose of the if statement is to check a boolean. Since dateRegex.test(fullDate) already returns a boolean, you can just pass it directly to the if statement. Now, if you want to check if the boolean is false rather than true, you can simply use the logical NOT (aka negation) operator (!) at the beginning to flip it. It will turn false to true and vis versa. You can also just replace your else if with just an else because if it's not false, then it has to be true.

if (!dateRegex.test(fullDate)) {
  isDOBValid;
} else if (dateRegex.test(fullDate) == true) {
  isDOBValid = true;
}

Second, you can actually get rid of the if statement altogether in both instances. The line isDOBValid; is doing absolutely nothing. Since we only need the logic that's in the else statement, we can simply move this logic to the if statement then negate the condition like we did before. For the first if statement we can either add the logical NOT operator like we did before, or just invert all the conditions. I will be inverting all of the conditions just because I think it looks a bit nicer. Then for the second one we can just remove the operator we put there before.

if (
  day < 31 &&
  month < 12 &&
  (month != 2 && day <= 29) ||
  ((month != 4 && month != 6 && month != 9 && month != 11) && day <= 30)
) {
  isDOBValid = true;
}

if (dateRegex.test(fullDate)) {
  isDOBValid = true;
}

The Problem

The actual problem with your code also lies with your if statements. They aren't connected at all. You only have one boolean (isDOBValid) that both if statements are setting for two completely different conditions. Since it's executed from top to bottom, the second condition will take precedence. This means that no matter what the day/month/year is, your function will always return true if your regex successfully matches. To fix this, you need to connect them. This can be done a couple of ways.

First, you can chain them.

if (
  day < 31 &&
  month < 12 &&
  (month != 2 && day <= 29) ||
  ((month != 4 && month != 6 && month != 9 && month != 11) && day <= 30)
) {
  if (dateRegex.test(fullDate)) {
    isDOBValid = true;
  }
}

This ensures that the second condition is only ran if the first condition succeeds.

Second, you can check the variable that is being set.

if (
  day < 31 &&
  month < 12 &&
  (month != 2 && day <= 29) ||
  ((month != 4 && month != 6 && month != 9 && month != 11) && day <= 30)
) {
  isDOBValid = true;
}

if (dateRegex.test(fullDate) && isDOBValid) {
  isDOBValid = true;
}

This does effectively the same thing as the last one, just with less nesting.

Last, and by far the best solution, would be to ditch your regular expression and manually checking day/month combinations, and just use the built-in JavaScript methods to do it. The simplest way to do this is to use Date class. This class will catch any invalid dates, but we do still need to be careful about leap years. For example, if you were to run new Date("02/31/2016"), the date would still be parsed, but the parsed result would be 3/02/2016 not 02/31/2016. This is simple to do though, we can just check if the month it gave us is the same month we gave it. If it's not, the date is invalid. We can get the date using the Date.getDate() function. The month it gives us is 0-based though, so we need to add 1 to it to get the actual month. Another great part about this function is that if the date is invalid, it just gives us NaN which is also really easy to check for. So, using this class, all we need to do to make sure the date is valid is check the month. If it isn't NaN and is the same as the one we gave it, it's valid.

Now, we just need to convert what we have into a valid date format. This is really easy because we already have the separate month, day, and year, so we can just convert it to the MM/dd/yy format by simply concatenating it all with a / in between. Your entire function would be reduced to this:

function checkDateOfBirth(idNumber) {
  const year = idNumber.substring(0, 2);
  const month = idNumber.substring(2, 4);
  const day = idNumber.substring(4, 6);

  var date = new Date(month   "/"   day   "/"   year);
  return !isNaN(date.getMonth()) && date.getMonth()   1 == month;
}

console.log(checkDateOfBirth("160230"));
console.log(checkDateOfBirth("050625"));
console.log(checkDateOfBirth("100431"));

CodePudding user response：

I follow @jesse comment and it work fine. You need to check regex only if day, month and year are good.

console.log(checkDateOfBirth("890230"));

function checkDateOfBirth(idNumber) {
    const year = idNumber.substring(0, 2);
    const month = idNumber.substring(2, 4);
    const day = idNumber.substring(4, 6);
    let isDOBValid = false;
    const dateRegex = /^([12]\d{3}-(0[1-9]|1[0-2])-(0[1-9]|[12]\d|3[0-1]))$/;
    const cutOffDate = new Date().getFullYear() - 2000;
    const fullDate =
        (year > cutOffDate ? "19" : "20")   year   "-"   month   "-"   day;

    if (day > 31 || month > 12) {
        isDOBValid = false;
    } else if (month == 2 && day > 29) {
        isDOBValid = false;
    } else if ((month == (4 || 6 || 9 || 11)) && day > 30) {
        isDOBValid = false;
    } else {
        if (dateRegex.test(fullDate) == false) {
            isDOBValid;
        } else if (dateRegex.test(fullDate) == true) {
            isDOBValid = true;
        }
    }
    return isDOBValid;
}

I hope it can help you.