Доброго времени суток.
Попался в проекте примерно такой код
public funciton getModelByTypeAndId($type, $id) {
switch ($name) {
case 'type0':
$offer = $this->em->getRepository(Type0::class)->find($id);
break;
case 'type1':
$offer = $this->em->getRepository(Type1::class)->find($id);
break;
case 'type2':
$offer = $this->em->getRepository(Type2::class)->find($id);
break;
case 'newType':
$offer = $this->em->getRepository(NewType::class)->find($id);
break;
case 'newType2':
$offer = $this->em->getRepository(NewType2::class)->find($id);
break;
case 'model':
$offer = $this->em->getRepository(Model::class)->find($id);
break;
default:
$offer = null;
}
return $offer;
}
И по проекту такого много. И оно мне не нравится, громоздко и дублирование. И вот я решил отрефакторить
Сделал так
public funciton getModelByTypeAndId($type, $id) {
$classes = [
'type0' => Type0::class,
'type1' => Type1::class,
'type2' => Type2::class,
'newType' => NewType::class,
'newType2' => NewType2::class,
'NewType3' => NewType3::class,
];
if (empty($classes[$name])) {
return null;
}
return $this->em->getRepository($classes[$name])->find($id);
}
Интуиция подсказывает, что что-то не то)
Вот такой вариант вроде легче должен читаться, но тот что я выше привел самый лаконичный
public funciton getModelByTypeAndId($type, $id) {
switch ($type) {
case 'auto' : $entity = Type0::class; break;
case 'type1' : $entity = Type1::class; break;
case 'type2' : $entity = Type2::class; break;
case 'newType' : $entity = NewType::class; break;
case 'newType2': $entity = NewType2::class; break;
case 'newType3': $entity = NewType3::class; break;
default : return null;
}
return $this->em->getRepository($entity)->find($id);
}
Возможно вообще сама логика работы данного метода не оптимальная с точки зрения ООП. Из-за того что фронтенд (откуда это запрос приходит) спроектирован не лучшим образом.
В общем, посоветуйте пожалуйста, как красивее и семантически правильно поступать в таких случаях?
Возможно стоит вместо default : return null; выбрасывать Exception, а вшитые строки ('type1','newType' ...) вынести в сами классы, чтоб это были aliases классов, и, например, всем им сделать статический метод getAlias и интерфейс какой нибудь (IEntityWithAlias например), т.е. что-то такое
public funciton getModelByTypeAndId($type, $id) : IEntityWithAlias {
switch ($type) {
case Type0::getAlias() : $entity = Type0::class; break;
case Type1::getAlias() : $entity = Type1::class; break;
//и т.д.
default : throw new InvalidArgumentException('Unknown category type');
}
return $this->em->getRepository($entity)->find($id);
Или ещё такой вариант, но в нем тройная вложенность
$classes = [
Type1::class,
Type2::class,
Type3::class,
NewType1::class,
///и т.д.
];
foreach ($classes as $class) {
if ($class::getAlias() === $name) {
return $this->em->getRepository($class)->find($id);
}
}
throw new InvalidArgumentException('Unknown category type');
Ну и последний вопрос.
Откуда черпать информацию, когда задаюсь подобными вопросами, пока само с опытом не пришло и что делать, чтоб пришло?)