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.";
}
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
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
Get help from others!
Recent Questions
Recent Answers
© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP