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

Ответ 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;
}

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

Ответ 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, но я думаю, что это похоже.

Ответ 3

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

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

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

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

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

Ответ 4

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

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

Это по крайней мере немного чище...

Ответ 5

Что-то вроде этого:

$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];

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

Ответ 6

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