4
\$\begingroup\$

I'm quite new to Laravel, and I'm not sure what am I doing is the best practice.

I'd like to return JSON if the request is Ajax, or return a view otherwise. This is the way I made it, and it works fine, but I'm not sure if this is the best way(it seems long winded). I'd really like to hear your suggestions.

public function store()
{
    $input = Input::all();

    if(!$this->settings->fill($input)->isValid())
    {
        if ( Request::ajax() ){
            return $this->jsonFailure(array(
                'errors' => $this->settings->errors
            ));
        }
        else{
            return Redirect::route('admin.settings.create')
                ->withInput()
                ->withErrors($this->settings->errors);
        }
    }

    $this->settings->save();

    if ( Request::ajax() )
        return $this->jsonSuccess('success');
    else
        Redirect::route('admin.settings.index');
}
\$\endgroup\$
0
5
\$\begingroup\$

The only problem with this approach is that as you add more methods, you are going to end up with an awful lot of repeated if (Request::ajax()) ... blocks all over your code.

Here's what I do for my sites:

<?php
class ApiController extends Controller {
    protected function makeResponse($data, $isError = false, $redirectToIfNotAjax = '/') {
        if (Request::ajax()) {
            if ($isError) {
                return $this->jsonFailure(array(
                    'errors' => $data
                ));
            } else {
                return $this->jsonSuccess($data);
            }
        } else {
            if ($isError) {
                return Redirect::route($redirectToIfNotAjax)
                    ->withInput()
                    ->withErrors($data);
            } else {
                Redirect::route($redirectToIfNotAjax);
            }
        }
    }
}

(This is not exactly the same as what I am using, but it's pretty close.)

Then, inherit your controllers from ApiController instead of just Controller.

Here is what your example would look like:

public function store()
{
    $input = Input::all();

    if(!$this->settings->fill($input)->isValid())
    {
        return makeResponse($this->settings->errors, true, 'admin.settings.create')
    }

    $this->settings->save();

    return makeResponse('success', false, 'admin.settings.index')
}

Much cleaner!

\$\endgroup\$
0
4
\$\begingroup\$

If you follow Single Responsibility principle than your controller should not know how view is presented. I would rather create view presenter class in you app/Projectname/Presenters folder and than inject it thru constructor. That way your code would be cleaner and more testable.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Great, this is somewhat I had in mind. To put it in a dedicated class and use it everywhere you need \$\endgroup\$ – Filip Zelic Jul 24 '14 at 7:44
1
\$\begingroup\$

make two different methods and in the routes.php change one for get and another for post and do the ajax request with post, that way you can use the same URL and separate functionality.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ That sounds like significant unnecessary code duplication. \$\endgroup\$ – Moshe Katz Jul 24 '14 at 4:41

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.