рефакторинг этого блока - PullRequest
Новая
       11

рефакторинг этого блока

1 голос
/ 30 декабря 2010

Я рефакторинг кода, который не был написан мной. Этот блок устанавливает значение $val, но я хочу немного его очистить. Очевидно, я не могу использовать третичный оператор здесь. Какие еще способы сделать этот код чище? 

  if (isset($vars[$input])) {
       $val = $vars[$input];
  } elseif (isset($this->getI['io'])) {
       $val = $this->getI['io'];
  } elseif (isset($vars[5])) {
       $val = $vars[5];
  } else {
       $val = 10;
  }

Ответы [ 5 ]

2 голосов
/ 30 декабря 2010 
$val = 10;
if (isset($vars[$input])) {
    $val = $vars[$input];
} elseif (isset($this->getI['io'])) {
    $val = $this->getI['io'];
} elseif (isset($vars[5])) {
    $val = $vars[5];
}

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

1 голос
/ 30 декабря 2010

Боюсь, я не знаю php.Я предполагаю, что если вы передадите (скажем) $ vars [$ input] в функцию, то к тому времени, когда она станет параметром для функции, его установка будет истинной (если это не так, япопробуйте написать функцию, которая проверяет isset () по своему параметру, и установите $ val, если это так).Я нахожу еще, чтобы добавить сложности;Я стараюсь их избегать.В этом случае я бы написал функцию, которая вернула бы значение;тогда все мои elseif могут стать простыми if. 

f() {
    if (isset($vars[$input])) {
        return $vars[$input];
    }
    if (isset($this->getI['io'])) {
        return $this->getI['io'];
    }
    if (isset($vars[5])) {
        return $vars[5];
    }
    return 10;
}

И, конечно, в вашей вызывающей функции присвойте $ val результату этой функции.

1 голос
/ 30 декабря 2010

Пока вы разбираетесь в этом, выясните, что он делает, и добавьте несколько комментариев.

например, зачем назначать его магическому числу 10?может быть, контекст остальной части этого может пролить некоторый свет.

Что касается кода, вы не получите его проще, чем этот.

1 голос
/ 30 декабря 2010

Я не знаю ... кажется, что это довольно лаконично, как есть.

Если вы знаете, что он делает, он делает это хорошо, и он достаточно чист, чтобы вы могли понять это сновав будущем я говорю, не трогай это.

1 голос
/ 30 декабря 2010

По-моему, ваш пример настолько чист, насколько это возможно. Конечно, вы могли бы написать это как огромную однострочную строку, используя троичный оператор: 

$val = isset($vars[$input]) ? $vars[$input] : isset($this->getI['io'] ? $this->getI['io'] : isset($vars[5]) ? $vars[5] : 10;

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

...