Я только что написал ужасную функцию PHP, мне нужна помощь (elseif chain - switch?)

Question

Я делаю сайт, который определяет значение массива в зависимости от времени. Я написал этот ужасный (функциональный) сценарий, и мне интересно, если бы я мог сделать его более кратким. Я начал с оператора case / switch, но у меня возникли проблемы с работой нескольких условных выражений. Вот грязный поступок:

if ($now < november 18th) {
    $array_to_use = $home;
}
elseif (november 18th < $now && $now < november 21st ) {
    $array_to_use = $driving;
}
elseif (november 21st < $now && $now < november 22nd) {
    $array_to_use = $flying;
}
...
...
...
elseif (february 1st < $now) {
    $array_to_use = $arrived;
}
else {
    $array_to_use = $default;
}

Расписание на самом деле более сложное и содержит 13 elseif операторов. Может кто-нибудь подтвердить, что у меня только что был блок кодера и что есть лучший способ сделать это?

РЕДАКТИРОВАТЬ: Я изменил метки времени Unix на грубое реальное время, чтобы было легче понять, что я делаю (надеюсь)

РЕДАКТИРОВАТЬ 2: Пожалуйста, простите за сейчас сломанные часы Javascript, но это сайт, над которым я работаю:

Расписание .

Каждый массив основан на моем местоположении, и есть 15 "они в настоящее время", основанные на времени. Это небольшая проблемная область с известными временами начала / окончания, поэтому гибкость не является ключевой, просто все написано. Вы можете видеть, как время непрерывно, и за один раз необходимо выбрать только один массив строк.

Answer 1

Сначала, пожалуйста, пожалуйста, выньте свои жестко закодированные числа и поместите их в константы.

$FLIGHT_START_TIME = 1258956001;
$FLIGHT_END_TIME   = 1260511201;

Во-вторых, я бы сделал мини-функции для каждого из условий:

т.е.

function isFlying($time)
{
    return ( $FLIGHT_START_TIME < $time && $time < $FLIGHT_END_TIME );
}

В-третьих, возьмите весь свой набор условий и поместите его в функцию, чтобы получить текущее состояние, и замените вызовы функций:

function getStateArrayForTime($time)
{

   if (isDriving($time)
   {
       return $driving;
   }
   if ( isFlying($time) )
   {
        return $flying;
   }
...etc
}

И наконец, замените весь встроенный раздел кода своим единственным вызовом функции:

$currentState = getStateArrayForTime($now);

Как прокомментировали и другие авторы, на данный момент вы можете использовать функцию, управляемую таблицей данных, для возврата состояния, если вы знаете, что параметрами состояния будут только время начала и окончания:

поэтому замените реализацию getStateArrayForTime на:

function getStateArrayForTime ($time)
{
// 
$states = array (
    array("startTime" => 1258956001, "endTime" => 1260511201, "state" => $flying),
    array("startTime" => 1260511201, "endTime" => 1260517000, "state" => $driving),
..etc...
);
    foreach($states as $checkStateArray)
    {
        if($checkStateArray['startTime'] < $time && $time < $checkStateArray['endTime'])
        {
            return $checkStateArray['state'];
        }
    }
    return null;
}

Наконец, некоторые люди могут спросить: «Зачем делать вещи в таком порядке?» Я не могу претендовать на кредит вообще, кроме как в приложении, но у Мартина Фаулера есть замечательная книга под названием «Рефакторинг», которая объясняет, почему вы очищаете код по одному шагу за раз, и тестируете на каждом этапе пути, а затем, наконец заменяйте функции оптом, которые не имеют смысла, все время проверяя, что они функционально эквивалентны.

Answer 2

Это может быть излишним, но я бы сделал что-то подобное, чтобы поместить все диапазоны времени в одно чистое место:

@timeWindows = ({ start -> 0, end -> 1258783201, array -> $home },
                ... ,
                {start -> 1260511201, end -> MAXVAL, array -> $arrived});

, а затем цикл, подобный

$array_to_use = $default;

foreach (my $window in @timeWindows) {
   if (($now > $window->start) && ($now < $window->end)) {
       $array_to_use = $window->array;
       last;
   }
}

Извините, это на Perl, я не знаю PHP, но я думаю, что это похоже.

Answer 3

Вы можете поместить время и массив для использования в массив и зациклить их для выбора.

$Selctions = array(
    1258783201 => $Home,
    1258956001 => $Driving,
    1260511201 => $Flying,
    ...
    1260511201 => $Arriving
);

<b>// MUST SORT so that the checking will not skip</b>
ksort($Selction);
$TimeToUse = -1;
$Now       = ...;
foreach ($Selctions as $Time => $Array) {
    if ($Now < $Time) {
        $TimeToUse = $Time;
        break;
    }
}
$ArrayToUse = ($TimeToUse != -1) ? $Selctions[$TimeToUse] : $Default;

Этот метод можно использовать только в том случае, если время не имеет разрыва (один диапазон сразу за другим).

Надеюсь, это поможет.

Answer 4

Вы можете использовать оператор switch, делая что-то вроде этого:

switch (true)
{
    case $now < 1258783201:
        // your stuff
        break;
    case $now < 1258783201
        // more of your stuff
        break;
    //...
}

Это хоть немного чище ...

Answer 5

Возможно, вы захотите изучить шаблон команд; это также может помочь в этих обстоятельствах.

Answer 6

Примерно так:

$array_to_use = null;
$dispatch = array(1258783201, $home, 1258956001, $driving, ..., $arrived);
for ($i=0; i<count($dispatch); $i+=2) {
    if ($now<$dispatch[$i]) {
        $array_to_use = $dispatch[$i+1];
        break;
    }
}
if ($array_to_use==null) $array_to_use = $dispatch[count($dispatch)-1];

Вам также нужно подумать, нужно ли вам условие "<" или "<=". </p>