Как избежать if-операторов с объектно-ориентированным дизайном, PHP - PullRequest
9 голосов
/ 16 июня 2011

Я в основном создаю дисплейный модуль для созданной мной рекламной системы.

Я пытаюсь избежать следующей конструкции с повторяющимися операторами if.

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

<?php

class Ad { 
    public $adState = 'active'; 
} 

class AdWriter { 
    public function displayAd(Ad $ad, $viewmode = 'visitor') { 
        if ($viewmode =='visitor') { 
            if ($adState == 'active') {} 

            else if ($adState == 'paused') {} 

            else if ($adState == 'inactive') {} 

        } 

        else if ($viewmode = 'owner') { 
            if ($adState == 'active') {} 

            else if ($adState == 'paused') {} 

            else if ($adState == 'inactive') {} 
        } 

        else if ($viewmode == 'administrator') { 
            if ($adState == 'active') {} 

            else if ($adState == 'paused') {} 

            else if ($adState == 'inactive') {} 
        } 
    } 
}  

?>

Ответы [ 6 ]

12 голосов
/ 16 июня 2011

Примените рефакторинг Замените условное на полиморфизм и посмотрите на шаблон состояния .

4 голосов
/ 16 июня 2011

Вы можете создать Factory (Pattern) , используя переключатель viewmode, и создать конкретный Ad, реализующий интерфейс с простой функцией «display», например .

Псевдо пример:

class AdFactory { 

    public static function getAd($sType) {
        switch($sType) {
            case "AdOne":
                return new AdOne();
            case "AdTwo":
                return new AdTwo();
        }
    }
    throw new Exception("Unknown ad!");
}

class AdOne implement AdInterface {
    public function display() {
        // All that AdOne does when displaying.
    }
}

interface AdInterface {
    public function display() { }
}

$oAd1 = AdFactory::getAd('typeOne');
$oAd1->display();

$oAd2 = AdFactory::getAd('typeTwo');
$oAd2->display();
3 голосов
/ 16 июня 2011

Вместо передачи $ viewmode передайте объект, который инкапсулирует логику для этого viewmore, и вызовите его метод, который сделает эту работу.Таким образом вы избавитесь от необходимости в операторах if.

0 голосов
/ 16 июня 2011

В 8 главе этой книги вы можете найти очень подробный ответ на свой вопрос.

Вкратце: используйте Композиция или Фабрики.(см. ответ Уэсли ван Опдорпа).

Кроме того, избегайте использования строковых аргументов как перечислимых: $viewmode = 'visitor'
с этим аргументом, вам придется хранить в памяти все возможные значения этого аргумента.Или загляните в код функции, чтобы запомнить их.И эти значения являются строками - хорошее место для опечаток.Кроме того, будет очень сложно изменить значения в функции, поскольку все вызовы этого метода будут содержать жестко закодированные строки.
Использовать константы класса:

class AdWriter { 
const view_mode_visitor = 1;
...

Также, $adState - неправильный код,должно быть $ ad-> state.Но использование открытых полей - это тоже плохая практика :)

0 голосов
/ 16 июня 2011
  switch($viewmode){
    case "visitor":
        switch($adstate){
            case "active": 
                //statement
                break;
            case "paused": 
            break;
            case "inactive": 
            break;
        }
        break;
    case "owner":
        break;
    case "administrator":
        break;
}
0 голосов
/ 16 июня 2011

Я подкрадываюсь к StackOverflow на работе, поэтому у меня нет времени, чтобы написать подробный ответ обо всех ваших возможностях.

Но чтобы «привести в порядок» эти ifs, вы можете сделать это:

switch $viewmode {
  case 'visitor':
    your_code_here;
    break;
  case 'owner':
    your_code_here;
    break;
  default:
    will_run_if_nothing_above_matches;
    break;
}
Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...