Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm trying to make controllers skinny as possible. I use repositories for accessing the database and I use Services to do other stuff. In this case, I use it to insert a new post to the database. For validation I use Jeffery Way's validation package for Laravel.

PostController.php

<?php

use Laracasts\Validation\FormValidationException;

use Dnianas\Post\PostCreationService;
use Dnianas\Post\PostRepository;

use Dnianas\Forms\PostForm;

class PostController extends BaseController
{

    /*
     * Dnianas\Services\PostCreationService
     */
    protected $post;

    /*
    * The post repository
     */
    protected $posts;

    /**
     * @param PostForm $postForm
     * @param PostCreationService $post
     * @param PostRepository $postRepo
     */
    public function __construct(PostForm $postForm, PostCreationService $post, PostRepository $postRepo)
    {
        $this->posts = $postRepo;
        $this->postForm = $postForm;
        $this->post = $post;
    }

    public function index()
    {
        $this->beforeFilter('auth');
    }


    /**
     * @return \Illuminate\Http\JsonResponse
     */
    public function create()
    {
        // Get the input
        $input = Input::all();

        // Validate it
        try {
            $this->postForm->validate($input);
        } catch(FormValidationException $e) {
            return Response::json([
                'success' => 'false',
                'message' => 'You didn\'t enter anything, Post cannot be empty.'
            ]);
        }

        // Insert it to the database
        $post = $this->post->create($input, Auth::user()->id);

        // Get the html content from the view
        $html = View::make('posts.post', ['post_id' => $post->id])->render();

        // Return a message along with the html content
        return Response::json([
            'success' => 'true',
            'message' => 'Your post has been successfuly posted!',
            'post_html' => $html
        ]);

    }

PostForm.php

<?php namespace Dnianas\Forms;

use Laracasts\Validation\FormValidator;

class PostForm extends FormValidator
{
    /**
     * Validation rules for when the user creates a post
     * @var array
     */
    protected $rules = [
        'post_content' => 'required'
    ];

}

PostCreationService.php

namespace Dnianas\Post;

class PostCreationService 
{

    /**
     * @param $input
     * @param $user_id
     * @return \Post
     */
    public function create($input, $user_id)
    {
        // Validation passed to we insert it to our database
        $post = new \Post;
        $post->post_content = $input['post_content'];
        $post->posted_date  = \Carbon::now();
        $post->user_id      = $user_id;
        $post->save();

        return $post;
    }
}

Is this controller skinny enough? Can it get any skinnier? If so, then how?

share|improve this question
1  
You can use Laravel events while saving posts. This way you don't need to check validation in the controller. –  Bishal Paudel Dec 22 '14 at 4:36

1 Answer 1

Check out Dwight Watson's Validating Trait. I like to use it with:

protected $throwValidationExceptions = true;

Validation of Eloquent models occurs automatically according to the rules defined on the model and if it fails, a ValidatingException is thrown that you can catch further up the application stack. For example, I might put the following exception handler somewhere like app/global/start.php

App::error(function (\Watson\Validating\ValidationException $exception)
{
    Log::error($exception);
    // I prefer 'success' => 'false' to be expressed in the response code
    return Response::json([
        'message' => $exception->getErrors()->first(),
    ], 422); // Unprocessable entity
});

Them my controllers end up looking more like this:

public function create()
{
    $input = Input::all();
    // Validation occurs automatically during object creation
    $post = $this->post->create($input, Auth::user()->id);
    $html = View::make('posts.post', ['post_id' => $post->id])->render();

    // I prefer "success" to be expressed by the 200 OK response code
    return Response::json([
        'message' => 'Your post has been successfully posted!',
        'post_html' => $html
    ]); // not specifying a response code implies 200 OK

}

But it's really just a matter of personal preference at this point. Your code looks good to me.

tl;dr +1 would pull & merge

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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