2

It's my first post here as I understand that SO is a platform to find fixes and this is where to ask questions about more general questions. Correct me if I'm wrong

I'm working on project in Laravel and I started wondering where to put my code.
It feels like this would perfectly fit in my Item model, but I'v always put this kind of logic on controllers side.

public function addComment($text, $isLog = false) {
    $comment = new ItemComment;
    $comment->item_id= $this->id;
    $comment->user_id = auth()->id();
    $comment->is_log = $isLog;
    $comment->comment = $text;
    $comment->save();
}

Would this be a good or bad practice to put this in Item and call it like $item->addComment(...);
I also have a ItemComment model, but something like $item->comments()->add(...) wouldn't work as ->comments() returns relationship.

2
1
+50

There is a school of thought that controllers and actions should be as lean as possible - pushing domain/business logic back into the models as much as possible. There's another school of thought that prefers to be a little more up front and show what's happening in the controller.

Then there's the approach of keeping it simple and only passing functionality to the model when your controller action is starting to get complicated (or, if there are multiple pathways to creating a comment, for example - such as different controller actions, or jobs/queues, or commands).

There's nothing really to any of these is good or bad practice. While "cleaner" code is always a good objective - there's quite a range of preference on what is cleaner.


In your case, it looks like you have a simple relation between an Item and an ItemComment using BelongsTo and HasMany. I see nothing wrong with keeping this in the controller as it is and moving on to build the rest of your application.

Even when relationships get more complex (for example, in one of my applications I have an Element which relates to both a parent and a root entity), I still prefer to create the new entity in the controller (without saving) and pass that to the parent's addEntity, which is responsible for setting relationships and then saving the new entity, rather than pass in (validated) request parameters.

Another advantage to having something like addComment on your Item model is that you go one step further and make other models "commentable" by creating a trait that can be reused and extracting the addComment method to that trait. Although, for this to work out you'll need to use a polymorphic relationship.

I also try to use the standard convention naming and routing for actions.

// ItemCommentsController.php
// action for POST /item/{id}/comments
public function store(Item $item, Request $request)
{
    // I'm assuming some basic validation here.
    $validated = $request->validate([
        'text' => 'required|max:1023',
        'isLog' => 'sometimes|boolean',
    ]);

    // $validated will only contain 'text' and 'isLog'.
    // also, the validate method will return an error to your user if invalid
    // and the following code will not be run.
    $comment =  $item->addComment(new ItemComment($validated));

    return; // you'll want to send a response or redirect here
}

// Comment.php
class Comment extends Model
{
    // The attributes that can be mass assigned through constructor.
    protected $fillable = ['text', 'isLog'];

    // relationship to whatever the comment is attached to.
    // your comments table will need a parent_id and parent_type field.
    public function parent()
    {
        $this->morphTo();
    }
}

// Item.php
class Item extends Model
{
    // use the Commentable trait.
    // (I put traits in a Behaviours directory for namespacing).
    use \App\Behaviours\Commentable;
}

// Commentable.php
trait Commentable
{
    public function addComment(Comment $comment)
    {
        $comment->parent_id = $this->id;
        $comment->parent_type = static::class;
        $comment->user_id = auth()->id();

        return $comment->save();
    }

    // this assumes your comments table has parent_id and parent_type fields.
    public function comments()
    {
        $this->morphMany(static::class, 'parent');
    }
}

You can further lean out your controller action by creating a Form Request and injecting that instead of the generic Request object.

2
  • In reality this code if from large app where it was used in multiple places and was in trait before. Then I moved it to model because $item->addComment($text, $isLog) felt more readable to me than $this->addCommentToItem($item, $text, $isLog). I just never have really understood what code, besides relationships, should be in model – NoOorZ24 Feb 5 '20 at 7:21
  • Anything that makes sense to be with the model – HorusKol Feb 5 '20 at 7:57

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.