Rymercyble's avatar

using logic in controllers

Hello, I would be very happy if someone turns me to the right way. At the moment I'm using controllers probably in the wrong way (I'm quite not sure) When I need for example manage articles I'm using 1 controller for /articles (get,post) - list of all or filtered articles /article (get,post) - showing 1 whole specific article and often even more than 4 methods. Is that ok or should I divide it ? Also, I'm probably putting too much logic to my methods for example

public function people()
    {
        $search = Request::has('search') ? Request::get('search') : 0;
        if ($search) {
            if (Request::get('min_age') == ''){
                $min_age = '10';
            } else {
                $min_age = Request::get('min_age');
            }
            if (Request::get('max_age') == ''){
                $max_age = '120';
            } else {
                $max_age = Request::get('max_age');
            }

            $carbon = Carbon::today();
            $carbon_max = Carbon::today();
            $max_age += 1;
            $year_min = $carbon->year - $min_age;
            $year_max = $carbon_max->year - $max_age;
            $carbon_min = $carbon->setDate($year_min, $carbon->month, $carbon->day)->toDateString();
            $carbon_max = $carbon_max->setDate($year_max, $carbon->month, $carbon->day)->toDateString();

            $users = User::when(!empty(Request::get('country')),function ($query){
                $query->where('country', Country::where('name', Request::get('country'))->select('id')->first()->id);
            })->when(!empty(Request::get('physical_gender')),function ($query){
                $query->where('physical_gender', Physical_gender::where('name', Request::get('physical_gender'))->select('id')->first()->id);
            })->when(!empty(Request::get('language')),function ($query){
                $query->whereHas('languages', function($query){
                    $query->where('name', Request::get('language'));
                });
            })->when(!empty(Request::get('language_want')),function ($query){
                $query->whereHas('unknownLanguages', function($query){
                    $query->where('name', Request::get('language_want'));
                });
            })->whereBetween('date_of_birth', [$carbon_max, $carbon_min])
                ->select('nick_name','first_name','last_name','date_of_birth','hidden_id')
                ->get();
        } else {
            $users = Sentinel::getUserRepository()->select('nick_name','first_name','last_name','date_of_birth','hidden_id')->get();
        }

        return view('people', compact('users'));
    }

if this is wrong where i should store logic ?

0 likes
3 replies
jlrdw's avatar
jlrdw
Best Answer
Level 75

Put that either in model or a repository, or use scopes.

example from controller:

$pets = Pet::getPets($petsearch);

Model:

    public function scopegetPets($query, $petsearch = '')
    {
        $petsearch = $petsearch . "%";
        $query = Pet::where('petname', 'like', $petsearch);
        if (ChkAuth::userRole('admin') === false) {
            $userid = Auth::user()->id;
            $query->where('ownerid', '=', $userid);
        }
        $results = $query->orderBy('petname', 'asc')->paginate(5);
        return $results;
    }

Ignore the ChkAuth part, it is my custom code.

https://laravel.com/docs/5.8/eloquent#query-scopes

See here also:

https://github.com/bestmomo/laravel5-5-example

He has good example of repository.

siangboon's avatar

there is no right or wrong as long as you can get your results you want, but you may need to start consider about the re-usability and maintainability to make your work better, for example, the time format can reactor into a helper function so you can use it any where you want without rewrite the code repeatedly, and the long user query may refactor into scope filter or method.

Please or to participate in this conversation.