Где лучше качество кода?

С коллегой вечный холивар о качестве кода.
Вот один из примеров.
Оцените пожалуйста, как правильней писать в подобных кусочках)

1)
if (!$_SESSION['isMobileOnly']) {
    $opinionList = $dao->getOpinionList($bar['id']);
} else {
    $opinionList = $dao->getOpinionList($bar['id'], 3);
}


2)
$opinionList = $dao->getOpinionList($bar['id'], $_SESSION['isMobileOnly'] ? 3 : null);


3) Свой вариант)
  • Вопрос задан
  • 2036 просмотров
Решения вопроса 1
Maksclub
@Maksclub Куратор тега PHP
maksfedorov.ru
Все плохо
  • Глобальные переменные — зло
  • На лицо некий код, который и с сессией работает и с БД — явное нарушение общепринятных принципов, например SRP
  • $dao как инициализирована? Явно не через конструктор
  • Использование цифры, вот сиди гадай по ней — это плохо, используйте именованную константу. Например 3 — это некий статус для desktop, вот и назовите ее STATUS_DESKTOP, хотя далее...
  • вы обращаетесь к одному методу, который шибко умный — работает с флагами, тем самым нарушая SRP и сам содержит if/else, сделайте просто 2 раздельных метода:
    getMobileOpinionList()
    getDefaultOpinionList()

  • Предикат лучше вынести в отдельный метод isMobileOnly(), тк условия могут поменяться в нем, не будете же везде в if менять по всему проекту
Ответ написан
Пригласить эксперта
Ответы на вопрос 6
Immortal_pony
@Immortal_pony Куратор тега PHP
$deviceCode = $_SESSION['isMobileOnly'] ? 3 : null;
$opinionList = $dao->getOpinionList($bar['id'], $deviceCode);
Ответ написан
@dkar007
Первый вариант визуально проще разобрать, чем второй.
Ответ написан
Oбернуть сессию в объект, использовать типы везде, где это возможно.

См. https://symfony.com/doc/current/components/http_fo...

p.s. число 3 вынести в именованную константу.
Ответ написан
Adamos
@Adamos
В порядке увеличения энтропии:
$opinionList = $dao->getOpinionList(
    $bar['id'], 
    $_SESSION['isMobileOnly'] ? 3 : null
);
Ответ написан
MaxVetrov
@MaxVetrov
А если так

$opinionList = $dao->getOpinionList(
    $bar['id'], 
    3 * $_SESSION['isMobileOnly']
);

?
Ответ написан
Ни один! Что за число 3, почему не вынесена в константу, если она так важна. Как будто вы пишите только для себя..
Ответ написан
Ваш ответ на вопрос

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

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