Привет. Есть такой метод:

public function validate($label, $type, $validators, $data)
    {
        if ($validators != false)
        {
            foreach ($validators as $validator) {
                if ($validator == 'required')
                {
                    self::validateRequired($data, $label);
                }
                elseif($validator == 'future')
                {
                    self::validateFuture($data, $label);
                }
                elseif($validator == 'aboveZero')
                {
                    self::validateAboveZero($data, $label);
                }
            }
        }
    }

Переменная $validator содержит строку с названием валидации. В зависимости от содержимого $validator вызывается нужная функция. Я еще давно где то прочитал, что это некрасивое решение, и его нужно переделать в соответствии с ООП. Там еще говорилось что то про паттерны. Но блин не помню. С паттернами практически не знаком.

Так как же все таки 'правильно' переписать такой код?

че-то есть подозрения, ООП тут не причем.

public function validate($label, $type, $validators, $data)
{
	foreach ($validators as $validatorName) {
		$validator = ValidatorFactory::get($validatorName);
		$validator->setLabel($label);
		$validator->setData($data);
		$validator->check();
	}
}

_____________
Люди, имеющие низкий уровень квалификации, делают ошибочные выводы, принимают неудачные решения и при этом неспособны осознавать свои ошибки в силу низкого уровня своей квалификации

switch( $validator ) {
  case 'required': ....

    break;
....
}

switch( $validator ) {
  case 'required': ....

    break;
....
}

Идея такая, что если if должен быть в коде – то он должен быть там только один раз. Например, в ValidatorFactory::get().
А так как PHP это динамический язык – то технически и от if можно избавиться:

$validatorClassName = 'Validator' . ucfirst($validatorName);
return new $validatorClassName();

Или как-нибудь с помощью магических методов. Может интервьюерам нужна была такая магия?

acerrusm,
Непонятно где находится твоя public function validate. Если оно встречается в коде только один раз – то по большому счету это ок. А если, например, в каждой в модели – то конечно не ок.

$validatorClassName = 'Validator' . ucfirst($validatorName);
return new $validatorClassName();

Я бы сделал через динамическую реализацию!

$type = 'function'

if(method_exists($this, 'validate'.$type))
    self::'validate'.$type($data)

Zzepish, а если у тебя будет сотня валидаторов разных? Все в один класс запихнешь? =) Да еще и вспомогательные методы, если валидатор будет сложным. Такая дичайшая каша получится, ты в этом классе просто утонешь при малейшем дебаге/доработке.

Потом Вася захочет дописать свой валидатор к имеющейся коллекции и чего ж ему, бедолаге, делать!? Ты ж обикаешься!

Фабрика это и есть динамическая реализация, только с нормальной архитектурой. =) Прчем она здесь настолько явная, что тут же показывает уровень владения ООП. Фабрика один из самых популярных патторнов группы порождающих. Не менее известный, чем Singletone. Если его не знаешь, то скорее всего вообще ничего не знаешь о паттернах и не слышал никогда.

Ron

Zzepish, мы тут про ООП, если что.