Какой код выглядит грамотнее?

Question

[Zukrine]

Задумался, как правильнее будет написать код, чтобы в будущем гипотетическим коллегам было проще ориентироваться в моих каракулях. Тут запись короче, но визуально, как мне кажется, сложнее читается:

class ArmoryController extends Controller
{
    public function armory_all()
    {
        foreach (Armory::where('use', 0)->get() as $item) {
            dump($item);
        }
    }
}

А вот тут немного длинее:

class ArmoryController extends Controller
{
    public function armory_all()
    {
        $items = Armory::where('use', 0)->get();
        foreach ($items as $item) {
            dump($item);
        }
    }
}

Как лучше будет? Чтобы сразу запомнить и тренироваться.

Answer 1

[Алексей Уколов]

Второй, конечно. Потом у вас ещё одно условие появится и оно перестанет помещаться в одну строку.

Comments

[Zukrine]

Точно, "расширяемость", огромное спасибо!

[Andrey]

Согласно соглашению об именовании в Laravel у вас выбрано неправильное имя метода. Вместо
public function armory_all()
надо изменить на что-то типа (с учетом что значение я так понимаю должно возвращаться)
public function getAllArmory()

[Zukrine]

seven5674, и вам огромная благодарность! Лучше не использовать нижнее подчеркивание в названиях функций, а писать их "верблюжьим регистром"?

[Pavel Shvedov]

Zukrine, а просто посмотреть в каком регистре названы сущности в самом ларавел, не?

[Zukrine]

Pavel Shvedov, да, нашел

[Andrey]

Zukrine, просто наберите в поиске "соглашение об именовании в Laravel"

Ну а если говорить в целом то у вас ошибочный подход к проектированию. Если кратко то контроллер должен быть "тонким"

Почитайте вот это для примера
https://dev.to/jsf00/implement-crud-with-laravel-s...

[Zukrine]

seven5674, просто смотрю гайды на ютубе, почему-то все пишут в контроллерах, хотя да, тоже читал, что контроллер должен быть простым, а вся логика в модель выносится. Пока не понял, как это сделать

[Алексей Уколов]

Говорить про тонкие контроллеры и прочую архитектуру имеет смысл только по отношению к конкретному коду конкретного проекта. В вопросе просто заготовка, обсуждать в ней нечего (кроме корректного указания на название метода). Во многих случаях логика в контроллере вполне приемлема, если она простая - это упрощает код и никак не усложняет написание тестов. Но, естественно, когда логика перестаёт быть максимально простой, тесты писать всё сложнее и это индикатор того, что пора "усложнять" архитектуру.

[Zukrine]

Алексей Уколов, спасибо большое за ответы и советы, буду работать в этом направлении, спасибо!

[Andrey]

Алексей Уколов, просто вопрос - а почему использования паттернов проектирования даже на "простых" проектах это "усложнять" ?

К тому же ТС еще в начале пути и не лучше ли будет ему сразу указать на то что есть другой, более правильный способ о котором его, к стати, я почти уверен будут долбать на собеседованиях под тем же соусом - контроллер должен быть тонким и не содержать в себе логики

В вашем постановке вопроса можно было его вообще просто отправить в route со словами - не парься

Я собственно упомянул про тонкие контроллеры только лишь потому что у него написано следующее - Controller

class ArmoryController extends Controller

Answer 2

[Антон Антон]

Совсем правильно может быть скоуп сделать https://laravel.com/docs/8.x/eloquent#local-scopes

Answer 3

[Вячеслав Плиско]

Напишите тесты и всё поймёте. Оба варианта ужасны.

Comments

[Zukrine]

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

[Вячеслав Плиско]

Zukrine, покройте код тестами и сами начнёте понимать где код неудачно реализован.

[Вячеслав Плиско]

tukbaevbr, не используются инъекции зависимостей. Зато используются статические вызовы, магические строчки ('use'), магические цифры (0) и цепочка вызовов - ничего из этого в контроллерах быть не должно от этого не только архитектура выиграет, но и код станет проще.

public function armory_all(Armory $armory) {
    return $armory->unused();
}

[Вячеслав Плиско]

tukbaevbr, если так не делать, то маленький проект превращается в большой и тродноподдерживаемый уже через месяц. Вот на примере простого теста уже вылезут косяки - тебе через костыль нужно замокать статический метод where, проверить что в аргументы приходят волшебная строка 'use' и 0, вернуть прокси объект с методом get. В итоге 4 костыля ради одной строчки кода.

И это лишь тесты, а в реальном проекте выльется в огромное дублирование кода, ошибки и трудности поддержки.

[Вячеслав Плиско]

tukbaevbr, смотря что тестируете. Если интеграционный тест - то не нужно. Если юнит - то не надо гонять базу, готовить миграции, фэктори и сиды для тестов. Это опять же к оценке качества кода через написание тестов. Простые тесты - значит код нормальный, его просто переиспользовать и саппортить.