TransWikia.com

Checking if an array of dates are within a date range

Code Review Asked on February 16, 2021

I created a dates_in_range() function which checks if all of the dates passed to it ($dates array) are within $start_date and $end_date. If they aren’t, it’ll return false.

The code works fine, but I’m wondering if there’s a way to refactor it better using something like array_filter? Or does this look fine?

function dates_in_range( $start_date, $end_date, $dates = array() ) {

    foreach ( $dates as $date ) {
        if ( ! ( ( (strtotime( $date ) >= strtotime( $start_date ) ) && ( strtotime( $date ) <= strtotime( $end_date ) ) ) ) ) {
            return false;
        }
    }

    return true;
}

$dates = array(
    '2021-11-19 00:00:00',
    '2020-11-20 00:00:00',
);

$start_date = '2021-10-01 00:00:00';
$end_date   = '2021-12-31 00:00:00';

// Returns true
if ( dates_in_range( $start_date, $end_date, $dates ) ) {
    echo "In range.";
} else {
    echo "Not in range.";
}

2 Answers

To me it looks fine, other than it makes me scroll the code to read it. On this ground I agree with Sam, that line ought to be shortened. To do so I would

  • use the fact that you don't need to convert mysql format dates to compare them
  • remove excessive bracing
  • use De Morgan's law to invert the condition

and get this

function dates_in_range( string $start_date, string $end_date, array $dates ): bool
{
    foreach ( $dates as $date ) {
        if ( $date < $start_date || $date > $end_date ) {
            return false;
        }
    }
    return true;
}

Also, I removed the default value for the $dates as I don't see any use in calling this function with only two parameters.

I am not sure about empty arrays though. Your function will return true for one, but it's hard to tell whether it's correct or not. It's like division by zero, and may be an exception must be thrown.

Answered by Your Common Sense on February 16, 2021

The function seems okay. In Javascript one could use the method Array.every(), but as this StackOverflow question illustrates there isn't really an equivalent method or functional approach in PHP that will stop the loop as soon as one element does not meet the requirement.

You might consider adhering to the PSR PHP Coding standards - especially PSR 12.

PSR-12 has a section about Lines:

2.3 Lines

There MUST NOT be a hard limit on line length.

The soft limit on line length MUST be 120 characters.

Lines SHOULD NOT be longer than 80 characters; lines longer than that SHOULD be split into multiple subsequent lines of no more than 80 characters each. 1

Readabillity suffers because the conditional line is a bit long, partly because of the indentation but also the additional spaces separating parentheses. It may not be an issue for anyone with a wide screen but as is visible in the code snippet one must scroll to the right to see the entire line. While it may only be a micro-optimization, one could pull out the strototime($date) out to a variable, which would not only decrease function calls but also allow for a shorter line. Similarly the calls to strtotime() for the start and end dates could be pulled out of the foreach loop and stored in variables so they only need to be calculated once per function call and can decrease the length of that line with the conditional.

I question whether the third parameter having a default of an empty array is appropriate. If the function is called without any argument, then an empty array will be used and the function will return true. Perhaps using type hinting would be a better technique:

function dates_in_range( string $start_date, string $end_date, array $dates) : bool {

Notice that method signature includes a return type declaration bool. This was added in PHP 7.0 2. Hopefully your code is running on PHP 7.3 or newer, since those are the version that are officially supported 3.

Answered by Sᴀᴍ Onᴇᴌᴀ on February 16, 2021

Add your own answers!

Ask a Question

Get help from others!

© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP