检查工作时间 - 改进代码/逻辑

Question

I have a system that checks if the current time is working hours in Hong Kong. I determined that working hours are from 9 AM to 6 PM. My code is working but i feel like there is a better/prettier way to write the IF condition.

The function will return True when the current time is NOT working time in HK. It will also return TRUE (as off hours in HK) 15 minutes before the end of the day (6 PM) and will return FALSE (as working hours in HK) 30 minutes before opening time.

function hkOffHours($endHour = 18, $startHour = 9) {
    $offHour = FALSE;

    // create the DateTimeZone object for later
    $dtzone = new DateTimeZone('Asia/Hong_Kong');

    // first convert the timestamp into a string representing the local time
    $time = date('r', time());

    // now create the DateTime object for this time
    $dtime = new DateTime($time);

    // convert this to the user's timezone using the DateTimeZone object
    $dtime->setTimeZone($dtzone);

    // print the time using your preferred format
    $time = $dtime->format('g:i A m/d/y');

    $hour = $dtime->format('G');
    $min  = $dtime->format('i');
    $day  = $dtime->format('D');

    // if weekend
    if(($day == 'Sun' || $day == 'Sat') )
    {
       $offHour = TRUE;
    }

    // if out of office hours
    if (
        ($admin_group == '1') &&
        (
           ( $hour == ($endHour-1) && $min>=45 ) ||
           ( $hour >= $endHour ) ||
           ( $hour == ($startHour-1) && $min <= 30 ) ||
           ( $hour <= ($startHour -2))
        )
    ) 
    {
        $offHour = TRUE;  
    } 

    return $offHour;        
} 

I will appreciate your thoughts.

Answer 1

Well, my first thought is you could refactor this by breaking it into several methods and a class, if you don't already do this that way.

For example:

    $offHour = FALSE;

    // create the DateTimeZone object for later
    $dtzone = new DateTimeZone('Asia/Hong_Kong');

    // first convert the timestamp into a string representing the local time
    $time = date('r', time());

    // now create the DateTime object for this time
    $dtime = new DateTime($time);

    // convert this to the user's timezone using the DateTimeZone object
    $dtime->setTimeZone($dtzone);

    // print the time using your preferred format
    $time = $dtime->format('g:i A m/d/y');

    $hour = $dtime->format('G');
    $min  = $dtime->format('i');
    $day  = $dtime->format('D');

All above fields could be class variables.

if(($day == 'Sun' || $day == 'Sat') )

this could be moved to separate function, called isWeekend()

if (
    ($admin_group == '1') &&
    (
       ( $hour == ($endHour-1) && $min>=45 ) ||
       ( $hour >= $endHour ) ||
       ( $hour == ($startHour-1) && $min <= 30 ) ||
       ( $hour <= ($startHour -2))
    )
) 
{
    $offHour = TRUE;  
}

this could be moved to isOutOfOfficeHours().

Whole class template:

class workingHours{

  private $dtzone;
  private ...
  (...)

  public function __construct($dtzone....){
    //setup values of class members
  }

  private function isWeekend(){} //see above

  private function isOutOfOfficeHours(){} //also see above

  public function hkOffHours(){
    return $this->isWeekend() || $this->isOutOfOfficeHours();
  }
}

Of course notice that I wrote this quick, I'm at work. Therefore my code logic might be not 100% compatible with yours. Anyway I hope that you'll get the idea.

Also one final thought - there are many books about refactoring. Try one! They'll make you a better coder.