Back

Search Function


Hello,

I am new here but I am not new to Laravel, however I am still learning and I want to improve. I created a "Helper" function called "Search" which I put in App\Library. The code works, but I would like to know how I could improve this code, because I think this is just average code at best. Any tips or advice would be welcome!

AppointmentsController:

public function index($order = 'date', $by = 'desc')
{
    if ($by == 'asc') $sort = "desc"; else $sort = "asc";

    $search = Input::get('search');

    $searchResult = Search::dateSearch($search);

    $number = $searchResult['number'];
    $month = $searchResult['month'];

    $appointments = Appointment::orderBy($order, $by)->where(function ($query) use ($search, $number, $month)
    {
        $query->where('appointment', 'LIKE', '%'. $search .'%')
            ->orWhere('date', 'LIKE', '%2015-' . $month . '-' . $number . '%')->get();
    })->Paginate(25);

    return view('appointments.index', compact('appointments', 'sort', 'search'));
}

Search Helper:

public static function dateSearch($searchString)
{
    $months = array("january", "february", "march", "april", "may", "june", "july", "august", "september", "october", "november", "december", "jan", "feb", "mar", "apr", "may", "jun", "jul", "aug", "sep", "oct", "nov", "dec");

    $search = strtolower($searchString);
    $search = trim($search);
    $search = explode(' ', $search);

    $number = "";
    $month = "";

    if (in_array($search[0], $months)) {
        $month = date('m', strtotime($search[0]));
    }

    if (is_numeric($search[0])){
        if (isset($search[1])) {
            if (in_array($search[1], $months)) {
                $number = str_pad($search[0], 2, '0', STR_PAD_LEFT);
                $month = date('m', strtotime($search[1]));
            }
        }
    }

return array('number' => $number, 'month' => $month);
}
thomastkim replied 3 years ago

I think there are ways to improve it, but can you start by telling us what you are actually passing through as your arguments?

RonB1985 replied 3 years ago

I have already improved it myself a bit (the controller code), edited my post.

What I am passing through is either a simple search for an appointment name, or (and what this helper is about) a month or a number and a month. For example, if I seach for "october" or "oct", I want all the appointments for october to be displayed. When I search for "1 oct" or "1 october" for example, I want all the appointments for the first of october to be displayed.

thomastkim replied 3 years ago

Hmm...Did you incorrectly copy/paste it? There is no $order or $by variables, but you are ordering by those two. There is also no sort, but you are returning that.

RonB1985 replied 3 years ago

Ah yeah, I left those out because my main question was about the search function, but I've edited my post and put them back in.

thomastkim replied 3 years ago Solution

Here's what I would change. These are just suggestions, and I haven't tested it out so let me know if anything doesn't work or if you don't agree with any changes. :)

First, I would just rename $by to $sort and remove the first if-else statement.

public function index($order = 'date', $sort = 'desc')
{
    // Remove this next line
    // if ($by == 'asc') $sort = "desc"; else $sort = "asc";

I don't see a point in the if-else statement since you can just pass the argument directly.

After that, if you use Carbon, you can actually really simplify the searching function into about 5 lines of code. For example (explanation below)...

public static function dateSearch($searchString)
{
    $searchString = trim($searchString);

    if (strpos($searchString, ' ') !== false)
    {
    	return Carbon::parse($searchString)->format('m-d');
    }
	
    return Carbon::parse($searchString)->format('m-');
}

You trim the string like you did before. If the string then contains a space, we return a m-d format, which would be something like "10-21" for October 21 (today's date).

Otherwise, we return the "m-" format, which is something like "10-" for October.

The Carbon::parse method will correctly parse "october", "1 october", etc. It takes care of all that for you, and Carbon already comes with Laravel. Then, since you are returning just a string and not returning an array (I didn't see why you needed to return an array), you could take out even more lines of code and just do this:

$searchResult = Search::dateSearch($search);

$appointments = Appointment::orderBy($order, $sort)->where(function ($query) use ($search, $searchResult)
{
    $query->where('appointment', 'LIKE', '%'. $search .'%')
        ->orWhere('date', 'LIKE', '%2015-' . $searchResult . '%')->get();
})->Paginate(25);

return view('appointments.index', compact('appointments', 'sort', 'search'));

You directly pass the $searchResult to the closure and search by that, which will be either "10-" if you just searched for the month or "10-21" if you searched for both the month and day.

RonB1985 replied 3 years ago

Thanks, I used Carbon before for other things but I never knew these things, I can look into it and learn a whole lot more. I will let you know how things go :)

Edit: Awesome stuff, it's working just the way you wrote it. It's pretty neat and simple and I love it. Thanks for the advice and the tips you gave me. One thing I would like to mention though.

In my view, I have the following to sort results either descending or ascending:

<th onclick="location.href='{{ URL::route('appointments.order', array('order' => 'appointment', 'by' => $sort, 'search' => $search )) }}'"><p>Appointment</p></th>

So, by default, my controller returns "desc" for the $sort. Once clicked on the link, my controller gets the $sort back, which is then "desc", so that is why I have the if statement, to change the $sort in my controller to "asc" and then return it into the view. Too complicated? I would love to know a better way ofcourse :)

Edit2: Ah, I see ofcourse there is a parse error once the search is just a simple word like "test", because carbon cannot parse it. So I need to check wether or not the search string is an actual date like "jan" or "january" for example. But I can manage that :) Your suggestions are already helping a lot.

thomastkim replied 3 years ago

Oh okay. Yea, I think changing the sort field would work then. Another possible way is to use JavaScript to alter it, but either way works.


Sign in to participate in this thread!



We'd like to thank these amazing companies for supporting us