@Gogeo

Array vs Switch case, code style best practice?

Доброго времени суток.
Попался в проекте примерно такой код
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');

Ну и последний вопрос. Откуда черпать информацию, когда задаюсь подобными вопросами, пока само с опытом не пришло и что делать, чтоб пришло?)
  • Вопрос задан
  • 126 просмотров
Пригласить эксперта
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Войти через центр авторизации
Похожие вопросы