Laravel controller method that searches jobs using variable criteria

Question

I have custom code where I check request params for existing and make query to database with model

public function search(Request $request)
{
    $position = $request->get("position") ?? null;
    $location = $request->get("location") ?? null;
    $employment = $request->get("employment") ?? null;

    $jobsBy = null;
    $jobs = null;
    if($position) {
        $jobs = Job::where('title', 'LIKE', '%'.$position.'%');

        if($location) {
            $locations = [];
            if(is_array($location)) {
                foreach ($location as $name) {
                    $locations[] = $name;
                }
            } else $locations[] = $location;
            $jobs = $jobs->whereMetaIn("location", $locations);
        }

        if($employment) {
            $employments = [];
            if(is_array($employment)) {
                foreach ($employment as $name) {
                    $employments[] = $name;
                }
            } else $employments[] = $employment;
            $jobs = $jobs->withAnyTag($employments);
        }

        if($jobs->get()->count()) $jobsBy = "position";
    }
    if($location && $jobsBy === null) {
        $locations = [];
        if(is_array($location)) {
            foreach ($location as $name) {
                $locations[] = $name;
            }
        } else $locations[] = $location;
        $jobs = Job::whereMetaIn('location', $locations);

        if($employment) {
            $employments = [];
            if(is_array($employment)) {
                foreach ($employment as $name) {
                    $employments[] = $name;
                }
            } else $employments[] = $employment;
            $jobs = $jobs->withAnyTag($employments);
        }

        if($jobs->get()->count()) $jobsBy = "location";
    }
    if($employment && $jobsBy === null) {
        $employments = [];
        if(is_array($employment)) {
            foreach ($employment as $name) {
                $employments[] = $name;
            }
        } else $employments[] = $employment;
        $jobs = Job::withAnyTag($employments);

        if($jobs->get()->count()) $jobsBy = "employment";
    }

    if($jobs == null || !$jobs->get()->count()) {
        $jobs = Job::where("status", '<', 2);
        $jobsBy = "latest";
    }

    if(isset($jobs) && $jobs instanceof \Illuminate\Database\Eloquent\Builder && $jobsBy != null) {
        $IDS = $jobs->where('status', '<', 2)->pluck('id');

        $partnerCompanyJobs = Job::with(['user' => function($user) {
            $user->without('roles', 'companies')
                 ->select('id', 'name');
        }])->withCount('views')->where('type', 0)
                               ->whereIn('id', $IDS)
                               ->where('status', 1)
                               ->orderBy('updated_at', 'DESC')
                               ->get();

        $companyJobs = Job::with(['user' => function($user) {
            $user->without('roles', 'companies')
                 ->select('id', 'name');
        }])->withCount('views')->where('type', 1)
                               ->where('status', 1)
                               ->whereIn('id', $IDS)
                               ->orderBy('updated_at', 'DESC')
                               ->get();

        $sampleJobs = Job::with(['user' => function($user) {
            $user->without('roles', 'companies')
                 ->select('id', 'name');
        }])->withCount('views')->where('type', 2)
                               ->where('status', 1)
                               ->whereIn('id', $IDS)
                               ->orderBy('updated_at', 'DESC')
                               ->get();

        $somonJobs = Job::withCount('views')->whereIn('type', [3,4])
                               ->whereIn('id', $IDS)
                               ->where('status', 1)
                               ->orderBy('updated_at', 'DESC')
                               ->get();

        $jobs = $partnerCompanyJobs->merge($companyJobs)
                               ->merge($sampleJobs)
                               ->merge($somonJobs);

        if(count($jobs)) {
            $page = $request->page ?? 1;
            $items = $jobs->forPage($page, 5);
            return response()->json([          
                'data' => JobResource::collection($items),
                'total' => count($jobs),
                'jobsBy' => $jobsBy
            ]);
        }
    }

    return response()->json([          
        'data' => [],
        'total' => 0,
        'jobsBy' => ""
    ]);
}

How can I avoid duplication of code here and create an optimal database query in my case? I have a problem processing request parameters. Because of this, much is duplicated in my code. How can I fix such a case?

Answer 1

I'd like to recommend condensing some of your script so that it is easier on the eyes.

This:

if($location) {
    $locations = [];
    if(is_array($location)) {
        foreach ($location as $name) {
            $locations[] = $name;
        }
    } else $locations[] = $location;
    $jobs = $jobs->whereMetaIn("location", $locations);
}

if($employment) {
    $employments = [];
    if(is_array($employment)) {
        foreach ($employment as $name) {
            $employments[] = $name;
        }
    } else $employments[] = $employment;
    $jobs = $jobs->withAnyTag($employments);
}

is more simply written as:

if ($location) {
    $jobs = $jobs->whereMetaIn("location", (array)$location);
}

if ($employment) {
    $jobs = $jobs->withAnyTag((array)$employment);
}

because casting the input as an array will convert a string into a single-element array. If the variable is already an array, then nothing changes.

But do you really want to overwrite the $jobs data formed from $location, if $employment is truthy? Either way that you answer that question, I think an elseif() is in order.

Please be precise with your conditionals. $jobs == null is more concisely written as !$jobs, but is not the same as $jobs === null. Likewise regarding $jobsBy != null.

Since $jobs is unconditionally declared, you can omit the isset() check.

Unless I misunderstand, it also feels like a good idea to execute and cache the following as a variable so that it can be used multiple times:

['user' => function($user) { $user->without('roles', 'companies')->select('id', 'name'); }]

You should also cache count($jobs) before or within your conditional, so that you are not making the same count() call twice.

---

Some more thoughts after initially posting my review:

1. withAnyTag() will happily accept mixed data (string or array).
   https://github.com/rtconner/laravel-tagging/blob/laravel-5/docs/usage-examples.md

2. When executing method calls on an object, just use the arrow syntax without =. In other words, you don't need to re-declare $jobs each time.

I don't have a Laravel project to play/test with (so there is no guarantee that this will work seamlessly, just consider it a collection of suggestions), but these are some of my thoughts on a script rewrite:

public function search(Request $request)
{
    $position = $request->get("position") ?? null;
    $location = $request->get("location") ?? null;
    $employment = $request->get("employment") ?? null;

    $jobsBy = null;
    $jobs = null;
    if ($position) {
        $jobs = Job::where('title', 'LIKE', '%' . $position . '%');
        $jobs->where('status', '<', 2);
        if ($location) {
             $jobs->whereMetaIn("location", (array)$location);
        }
        if ($employment) {
            $jobs->withAnyTag($employment);
        }
        if ($jobs->get()->count()) {
            $jobsBy = "position";
        }
    }

    // if no jobs by position, try with location...
    if ($location && !$jobsBy) {
        $jobs = Job::whereMetaIn('location', (array)$location);
        $jobs->where('status', '<', 2);
        if ($employment) {
             $jobs->withAnyTag($employment);
        }
        if ($jobs->get()->count()) {
            $jobsBy = "location";
        }
    }

    // if no jobs by location, try with employment...
    if ($employment && !$jobsBy) {
        $jobs = Job::withAnyTag($employment);
        $jobs->where('status', '<', 2);
        if ($jobs->get()->count()) {
            $jobsBy = "employment";
        }
    }

    // if not jobs, fallback to latest jobs...
    if (!$jobsBy) {
        $jobs = Job::where("status", '<', 2);
        $jobsBy = "latest";
    }

    if ($jobs !== null && $jobs instanceof \Illuminate\Database\Eloquent\Builder) {
        $ids = $jobs->pluck('id');

        $userFilter = ['user' => function($user) {
            $user->without('roles', 'companies')
                 ->select('id', 'name');
        }];

        $partnerCompanyJobs = Job::with($userFilter)
            ->withCount('views')
            ->where('type', 0)
            ->whereIn('id', $ids)
            ->where('status', 1)
            ->orderBy('updated_at', 'DESC')
            ->get();

        $companyJobs = Job::with($userFilter)
            ->withCount('views')->where('type', 1)
            ->where('status', 1)
            ->whereIn('id', $ids)
            ->orderBy('updated_at', 'DESC')
            ->get();

        $sampleJobs = Job::with($userFilter)
            ->withCount('views')->where('type', 2)
            ->where('status', 1)
            ->whereIn('id', $ids)
            ->orderBy('updated_at', 'DESC')
            ->get();

        $somonJobs = Job::withCount('views')
            ->whereIn('type', [3, 4])
            ->whereIn('id', $ids)
            ->where('status', 1)
            ->orderBy('updated_at', 'DESC')
            ->get();

        $jobs = $partnerCompanyJobs
            ->merge($companyJobs)
            ->merge($sampleJobs)
            ->merge($somonJobs);

        $jobCount = count($jobs);
        if ($jobCount) {
            $page = $request->page ?? 1;
            $items = $jobs->forPage($page, 5);
            return response()->json([
                'data' => JobResource::collection($items),
                'total' => $jobCount,
                'jobsBy' => $jobsBy
            ]);
        }
    }

    return response()->json([
        'data' => [],
        'total' => 0,
        'jobsBy' => ""
    ]);
}