Нормальный ли это код? и как его можно оптимизировать (Фильтр данных из базы)?

Question

[Alexander]

В руки попался вот такой здаровенный код:

Длинный код

public static function filter($visa_type, $status, $company)
    {
        if (Auth::user()->hasRole('administrator|Manager')) {
            if ($visa_type && $status & $company) {
                return self::where('visa_type', $visa_type)
                    ->where('status', $status)
                    ->where('company', $company)
                    ->orderBy('id', 'desc')
                    ->paginate(40)
                    ->toArray()['data'];
            } else if ($visa_type && $status) {
                return self::where('visa_type', $visa_type)
                    ->where('status', $status)
                    ->orderBy('id', 'desc')
                    ->paginate(40)
                    ->toArray()['data'];
            } else if ($visa_type && $company) {
                return self::where('visa_type', $visa_type)
                    ->where('status', $status)
                    ->orderBy('id', 'desc')
                    ->paginate(40)
                    ->toArray()['data'];
            } else if ($status && $company) {
                return self::where('company', $company)
                    ->where('status', $status)
                    ->orderBy('id', 'desc')
                    ->paginate(40)
                    ->toArray()['data'];
            } else if ($visa_type) {
                return self::where('visa_type', $visa_type)
                    ->orderBy('id', 'desc')
                    ->paginate(40)
                    ->toArray()['data'];
            } else if ($status) {
                return self::where('status', $status)
                    ->orderBy('id', 'desc')
                    ->paginate(40)
                    ->toArray()['data'];
            } else {
                return self::where('company', $company)
                    ->orderBy('id', 'desc')
                    ->paginate(40)
                    ->toArray()['data'];
            }
        } else {
            if ($visa_type && $status & $company) {
                return self::where('user_id', Auth::user()->id)
                    ->where('visa_type', $visa_type)
                    ->where('status', $status)
                    ->where('company', $company)
                    ->orderBy('id', 'desc')
                    ->paginate(40)
                    ->toArray()['data'];
            } else if ($visa_type && $status) {
                return self::where('user_id', Auth::user()->id)
                    ->where('visa_type', $visa_type)
                    ->where('status', $status)
                    ->orderBy('id', 'desc')
                    ->paginate(40)
                    ->toArray()['data'];
            } else if ($visa_type && $company) {
                return self::where('user_id', Auth::user()->id)
                    ->where('visa_type', $visa_type)
                    ->where('status', $status)
                    ->orderBy('id', 'desc')
                    ->paginate(40)
                    ->toArray()['data'];
            } else if ($status && $company) {
                return self::where('user_id', Auth::user()->id)
                    ->where('company', $company)
                    ->where('status', $status)
                    ->orderBy('id', 'desc')
                    ->paginate(40)
                    ->toArray()['data'];
            } else if ($visa_type) {
                return self::where('user_id', Auth::user()->id)
                    ->where('visa_type', $visa_type)
                    ->orderBy('id', 'desc')
                    ->paginate(40)
                    ->toArray()['data'];
            } else if ($status) {
                return self::where('user_id', Auth::user()->id)
                    ->where('status', $status)
                    ->orderBy('id', 'desc')
                    ->paginate(40)
                    ->toArray()['data'];
            } else {
                return self::where('user_id', Auth::user()->id)
                    ->where('company', $company)
                    ->orderBy('id', 'desc')
                    ->paginate(40)
                    ->toArray()['data'];
            }
        }
    }

Comments

[Станислав Почепко]

Эмм. Если хотите привести в порядок - то тут как бы не в laravel дело. Базовые знания PHP и веления кода.

[Сергей Протько]

Разберитесь с DRY.

Answer 1

[Andrzej Wielski]

Более резиновый вариант, используя коллекции и scope функции:

public function scopeFilter($query, $parameters = []){
    $fields = ['visa_type', 'status', 'company'];
    $filter = collect([
      'visa_type' => null,
      'status' => null,
      'company' => null,
      'paginate' => 40,
      'order' => 'id',
      'sort' => 'desc'
    ])->merge($parameters);

    if (!Auth::user()->hasRole('administrator|Manager')) {
      $query->where('user_id', Auth::user()->id);
    }
    
    foreach($filter->only($fields) as $field => $value){
      $query->where($field, $value);
    }

    return $query
      ->orderBy($filter->get('order'), $filter->get('sort'))
      ->paginate($filter->get('paginate'))
      ->toArray()['data'];
}

Использование -

SomeModel::where(...)->filter([
   'visa_type' => '....',
   'paginate' => 20,
   'sort' => 'asc'
]);

Answer 2

[yegreS]

нужно оптимизировать
Попробуйте что-то типа:

$dataProvider = new self(); //вот этот момент надо проверить
if ($visa_type) {
    $dataProvider->where('visa_type', $visa_type);
}
if ($status) {
    $dataProvider->where('status', $status);
}

...

return $dataProvider->orderBy('id', 'desc')
                    ->paginate(40)
                    ->toArray()['data']

Answer 3

[Сергей Протько]

У вас там баги и копипаста, как такой код может быть хорошим? Что вы будете делать если вам нужно будет в один прекрасный день выводить не 40 элементов а 50? find/replace? А если поменяется название поля? А если добавится еще одно поле? Добавлять степень двойки веток?

if ($visa_type && $status & $company) {

вот тут например я полагаю что мы имеем дело с опечаткой.

Дополню Кандрашкин Алексей и избавлюсь еще от одного уровня вложенности.

public static function filter($visa_type, $status, $company)
    {
        $query = self::query();

        if (!Auth::user()->hasRole('administrator|Manager')) {
            $query->where('user_id', Auth::user()->id);
        }
        if ($visa_type) {
            $query->where('visa_type', $visa_type);
        }
        if ($status) {
            $query->where('status', $visa_type);
        }
        if ($company) {
            $query->where('company', $company);
        }

        return $query
            ->orderBy('id', 'desc')
            ->paginate(40)
            ->toArray()['data'];