Using method trait to return something in laravel model

Question

I have question about scope yesterday, and someone gives an answer to don't return anything in scope. Is return something in scope really bad practice? Should I not return anything in the model except query?

Rather than scope, I want to create a normal method to return the select2 format. I have so many model/table that I need to return to select2 format, I think it's WET (write every time) if I must write every model or controller. I think I need to use DRY(Don't Repeat Yourself) principle, so I want to use a trait like below:

trait ModelTrait
{
    public static function getSelect2format($column = "name")
    {
        $data = static::select('id', $column.' as text')->orderBy($column, 'asc')->where($column, "like", "%".request()->q."%");

        if (is_array(request()->filter) && !empty(request()->filter)) {
            foreach(request()->filter as $key => $val)
            {
                if(in_array($key,$this->filterable))
                {
                    $data = $data->where($key, $val);
                }

            }
        }
        $data      = $data->limit(5)->get();

        return $data;
    }
}

I just need to call use ModelTrait in every model that I need and call it in the controller like below:

public function selectJson()
{

    $data = Customer::getSelect2Format("column_name");


    return \Response::json($data);
}

Is the code above break the best practice? What are the concerns or something that I need to improve?

Answer 1

I have question about scope yesterday, and someone gives an answer to don't return anything in scope. Is return something in scope really bad practice?

• This is because laravel is handling "it" for you.

• getSelect2Format is not the best name since you are not handling formatting but apply a specific set of rules relevant to select2 js plugin.

• I would suggest a repository class or a helper class to handle this task.

• Your code is messy, very long lines too, which is hard to read! I would suggest following PSR-12.

• I would suggest using response() or just return $data in controller class methods.

• I would also suggest early returns in your methods to reduce nesting hence easy reviewing by reviewers.

• suggest not using ->get() in a method unless you are absolutely certain on what method should do. i.e. you might want to apply a scope to this query later on.

• I suggest passing request data into the class to make it easier to test. Also, the less your class knows about other parts of the system the better.

• Dont you have to use DB::raw for $column.' as text'?

class select2Filters
{
    public static function apply(
        Model $model, 
        array $data, 
        string $column = 'name', 
        int $limit = 5
    ) {
        $filter = Arr::get($data, 'filter');
    
        return $model::selectRaw(['id', "{$column} as text"])
             ->where($column, 'like', "%{$data['q']}%")
             ->when(is_array($filter), function($query) use ($filter) {
                 $query->where(Arr::only($filter, $model::filterable)); 
             })
             ->orderBy($column, 'asc')
             ->limit($limit)
             ->get();
    }
}

I would also suggest adding an interface to each model that should have support for select2Filters. That interface should inforce filterable variable.

I would personally not use static, that way you can add configuration methods, i.e. setLimit(). OR use data struct class to contain data needed for select2Filters class.