- As s3rius pointed out, you don't need the
$timeCheck
variable. You can just use return true;
or return false;
.
- To parse a time use the DateTime class. You can find the supported time formats here. The constructor will throw an exception if it cannot parse the string. Consider if you want to handle the exception yourself or let the caller handle it.
- What if the start time is after the end time? You can throw an exception, switch the start and end times, or not handle the error.
- The meaning of the return value is not obvious based on your function name. How does it test the range? Change it to something like rangesNotOverlap or something similar so that it is apparent that true means they don't overlap.
- Are time ranges open or close intervals? If one range ends at 10:00 and the other starts at 10:00, do they overlap?
- Lets consider all cases:
.
1
Case 01: |-----|
|-----|
2
1
Case 02: |-----|
|-----|
2
1
Case 03: |-----|
|-----|
2
1
Case 04: |---------|
|-----|
2
1
Case 05: |-----|
|---------|
2
1
Case 06: |-----|
|-----|
2
1
Case 07: |-----|
|-----|
2
1
Case 08: |-----|
|-----|
2
1
Case 09: |-----|
|-----|
2
1
Case 10: |
|-----|
2
1
Case 11: |-----|
|
2
1
Case 12: |
|-----|
2
1
Case 13: |
|-----|
2
1
Case 14: |-----|
|
2
1
Case 15: |-----|
|
2
1
Case 16: |
|
2
| | Operlap | Overlap
Case | Example | Open Interval | Closed Interval
--------|-------------------------|---------------|-----------------
Case 01 | 09:00-11:00 09:00-11:00 | Yes | Yes
Case 02 | 09:00-11:00 10:00-12:00 | Yes | Yes
Case 03 | 10:00-12:00 09:00-11:00 | Yes | Yes
Case 04 | 09:00-12:00 10:00-11:00 | Yes | Yes
Case 05 | 10:00-11:00 09:00-12:00 | Yes | Yes
Case 06 | 09:00-10:00 11:00-12:00 | No | No
Case 07 | 11:00-12:00 09:00-10:00 | No | No
Case 08 | 09:00-10:00 10:00-11:00 | No | Yes
Case 09 | 10:00-11:00 09:00-10:00 | No | Yes
Case 10 | 10:00-10:00 09:00-11:00 | Yes | Yes
Case 11 | 09:00-11:00 10:00-10:00 | Yes | Yes
Case 12 | 09:00-09:00 09:00-10:00 | No | Yes
Case 13 | 10:00-10:00 09:00-10:00 | No | Yes
Case 14 | 09:00-10:00 09:00-09:00 | No | Yes
Case 15 | 09:00-10:00 10:00-10:00 | No | Yes
Case 16 | 09:00-09:00 09:00-09:00 | No | Yes
Your function returns the correct result on the closed interval for all cases.
- We can make your code simpler by noticing that we only have to check for two cases. The ranges don't overlap only if
end1 < start2
, (the same as your first if), or if end2 < start1
(which is included in your second if). The other check you do start1 > start2
is redundant, because it is always true if end2 < start1
.
Put it all together:
function rangesNotOverlapClosed($start_time1,$end_time1,$start_time2,$end_time2)
{
$utc = new DateTimeZone('UTC');
$start1 = new DateTime($start_time1,$utc);
$end1 = new DateTime($end_time1,$utc);
if($end1 < $start1)
throw new Exception('Range is negative.');
$start2 = new DateTime($start_time2,$utc);
$end2 = new DateTime($end_time2,$utc);
if($end2 < $start2)
throw new Exception('Range is negative.');
return ($end1 < $start2) || ($end2 < $start1);
}
function rangesNotOverlapOpen($start_time1,$end_time1,$start_time2,$end_time2)
{
$utc = new DateTimeZone('UTC');
$start1 = new DateTime($start_time1,$utc);
$end1 = new DateTime($end_time1,$utc);
if($end1 < $start1)
throw new Exception('Range is negative.');
$start2 = new DateTime($start_time2,$utc);
$end2 = new DateTime($end_time2,$utc);
if($end2 < $start2)
throw new Exception('Range is negative.');
return ($end1 <= $start2) || ($end2 <= $start1);
}
$timeCheck
variable. You can just writereturn true
orreturn false
instead of using that variable. That saves you a few lines of excess code. \$\endgroup\$