2
\$\begingroup\$

I have a method that checks if a date is correct or not, which returns a boolean if it is or not. In the method, I first check the format, then the date by using the function checkdate, then I check that time is of the format His.

/**
  * Function that checks the format, date, and time of a given timestamp.
  * @author : somar
  * @param string $date timestamp ex: 20161230154523
  * @return boolean resultat de check
  */
public function validateDate($date)
{
    if (preg_match("/^(((\d{4}((0[13578]|1[02])(0[1-9]|[12]\d|3[01])|(0[13456789]|1[012])(0[1-9]|[12]\d|30)|02(0[1-9]|1\d|2[0-8])))|((\d{2}[02468][048]|\d{2}[13579][26]))0229)){0,8}$/", substr($date, 0, 8))) {
        $formatCheck = true ;
    } else {
        $formatCheck = false ;
    }
    $dateCheck = checkdate(substr($date, 4, 2), substr($date, 6, 2), substr($date, 0, 4));
    $timeCheck = substr($date, 8, 2) <= 23 && substr($date, 10, 2) <= 59 && substr($date, 12, 2) <= 59;
    if ($dateCheck == true && $timeCheck == true && $formatCheck == true) {
        return true;
    } else {
        return false;
    }
}

I want to improve the performance, by reducing the amount of function calls and by using less checking instructions. Moreover, PHP prefers to not use regex, and I don't know how a workaround.

\$\endgroup\$
0
3
\$\begingroup\$

The complex regex is not really usefull here, you could simplified as:

if (preg_match("/^\d{8}$/", substr($date, 0, 8))) {

It is enough to test the length.

Then, if it doesn't match, return immediately:

if ( !preg_match("/^\d{8}$/", substr($date, 0, 8))) return false;

then call checkdate and return a false after call if the result is false:

if ( !checkdate(substr($date, 4, 2), substr($date, 6, 2), substr($date, 0, 4)) ) return false;

if ( substr($date, 8, 2) > 23 || substr($date, 10, 2) > 59 || substr($date, 12, 2) > 59) return false;

return true;

Edit according to comment:

if ( !preg_match("/^\d{8}$/", substr($date, 0, 8))) {
    return false;
}
if ( !checkdate(substr($date, 4, 2), substr($date, 6, 2), substr($date, 0, 4)) ) {
    return false;
}
if ( substr($date, 8, 2) > 23 || substr($date, 10, 2) > 59 || substr($date, 12, 2) > 59) {
    return false;
}
return true;

Note: take care that sometimes, the number of seconds can be greater than 59. That is called leap second

\$\endgroup\$
4
  • \$\begingroup\$ This is really helpful, but as I am using linter-phpcs (codesniffer), it is telling me that inline control structures are not allowed :( \$\endgroup\$
    – Somar
    Feb 13 '17 at 10:12
  • \$\begingroup\$ @Somar: see my edit. \$\endgroup\$
    – Toto
    Feb 13 '17 at 10:16
  • \$\begingroup\$ thank you. Are you sure this is not going to always return true ? \$\endgroup\$
    – Somar
    Feb 13 '17 at 10:19
  • \$\begingroup\$ @Somar: Yes, I am. \$\endgroup\$
    – Toto
    Feb 13 '17 at 10:19
4
\$\begingroup\$

The reason PHP dislikes the use of Regex is because some people don't know how to use it. You, for example, don't. Regex is for matching input, not for validation. And so you'd use it to change say your date format to ISO 8601:

> $date = '19700101000000';
> preg_replace('/(\d{4})(\d{2})(\d{2})(\d{2})(\d{2})(\d{2})/', '$1-$2-$3T$4:$5:$6Z', $date);
=> "1970-01-01T00:00:00Z"

The regex is simple as we know the data should be there. Not that it's correct.

Your code is also long-winded, rather than manually checking if the code is alright make PHP do it for you with strptime. You know the format the date is in, %Y%m%d%H%M%S, and so you can use it:

> strptime($date, '%Y%m%d%H%M%S');
=> {"tm_sec":0,"tm_min":0,"tm_hour":0,"tm_mday":1,"tm_mon":0,"tm_year":70,"tm_wday":4,"tm_yday":0,"unparsed":""}

And so you could change the function to just use:

/**
  * Function that checks the format, date, and time of a given timestamp.
  * @author : somar
  * @param string $date timestamp ex: 20161230154523
  * @return boolean resultat de check
  */
public function validateDate($date)
{
    return (bool)strptime($date, '%Y%m%d%H%M%S');
}

To note, strptime seems to have a bug with %S:

> strptime('19700101000070', '%Y%m%d%H%M%S');
=> {"tm_sec":7,"tm_min":0,"tm_hour":0,"tm_mday":1,"tm_mon":0,"tm_year":70,"tm_wday":4,"tm_yday":0,"unparsed":"0"}

To fix this you could change the function to take ISO 8601 dates, and have a function that converts to this format.

/**
  * Function that checks the format, date, and time of a given ISO 8601 timestamp.
  * @author : somar
  * @param string $date timestamp ex: 2016-12-30T15:45:23Z
  * @return boolean resultat de check
  */
public function validateDate($date)
{
    return (bool)strptime($date, '%Y-%m-%dT%H:%M:%SZ');
}

public function compactDateToISO($date)
{
    return preg_replace('/(\d{4})(\d{2})(\d{2})(\d{2})(\d{2})(\d{2})/', '$1-$2-$3T$4:$5:$6Z', $date);
}
\$\endgroup\$
0
3
\$\begingroup\$

I absolutely agree with the approach outlined by @Peilonrayz here of just trying to evaluate the input string directly as a date/time of known format vs. using regex. I would however suggest using DateTime::createFromFormat() to try to instantiate a more useful DateTime object upon success of the parsing operation (with false returned on failure).

That could eliminate the need for this function altogether. So instead of this usage:

$isValid = validateDate($inputString);

// you still need to do something else if you want to work with the date/time meaningfully

You could simply do

$dateTime = DateTime::createFromFormat('YmdHis', $inputString);

if (false === $dateTime) {
    // bad input
} else {
    // you can now work directly with $dateTime
}

I would also mention that you should be careful in referring to the input string as a timestamp, which is something that usually has connotation as a UNIX timestamp (i.e. seconds from UNIX epoch), not simply a particular date-time format.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.