2
\$\begingroup\$

I am using this code in my controller and would like to know if this is good code to use or if I can write it better, or more compact, and so on. I am just trying to learn from this. It does work, so that is not my question.

public function search()
{
    $search     = Input::get('search');
    $result     = Categories::where('name', 'LIKE', '%'. $search .'%')->get();

    if ($result->first()) {

        $categories     = Categories::where('name', 'LIKE', '%'. $search .'%')->Paginate(Auth::user()->categories);

        Session::flash('alertMessage', 'The below results are filtered due to a search action!');

        return View::make('categories.index', compact('categories'));
    }
    else {

        $categories = Categories::orderBy('name', 'asc')->Paginate(Auth::user()->categories);

        Session::flash('alertMessage', 'Your search has returned 0 results from the database.');

        return View::make('categories.index', compact('categories'));
    }
}
\$\endgroup\$
1
\$\begingroup\$

Here are few things I'd do to improve your code:

  • I would compact your first if statement into something such as if (Categories::where('name', 'LIKE', '%'. $search .'%')->first()) { since you only use $result for that purpose.
  • Remove that unnecessary horizontal space after your first assignment to $categories.
  • I hope that your flash messages are only for development purposes. They aren't very useful or user friendly as they are right now. Look at studied websites (Amazon, StackExchange, etc.) and UX.SE for tips on how to improve the user experience of your search function.
  • Be careful using the compact function. It's easy to lose track of it and forget to update it or your variables and therefore have something throw an error later on.
  • Fix the format of your else. Pick-and-choose either

    } else { 
    

    or

    }
    else
    {
    
  • Most search functions don't return anything when nothing was found. Again, this an issue to bring up at UX.SE, but your user might find themselves thinking that the results from your else clause are actually what they're looking for.

Overall, very nice. I'm sure you've done this, if not though, read through what you find from a search like this just to see how others have solved this problem. You'll find out good ways and bad ways of doing what you've done.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.