IMO, the second approach is only viable option as far as I concern.

One of the responsibilities of a repository is to persist your domain objects, meaning that it should only accept a domain object (already validated) for saving/updating.

$user = User::findOrFail($userId); should belong to an application layer.

moon0326 said:

IMO, the second approach is only viable option as far as I concern.

One of the responsibilities of a repository is to persist your domain objects, meaning that it should only accept a domain object (already validated) for saving/updating.

$user = User::findOrFail($userId); should belong to an application layer.

Dear moon0326, if I don't remember bad, the first function findOrFail is well used in the UserRepository because you are not using the internal Model object directly in the Application Layer.

Could you explain why the findOrFail shouldn't be in the Repository?

Thanks in advanced.

I think this is exactly why repositories are useful, look at below code

<?php namespace Acme;

class UserRepository implements UserRepositoryInterface {

    protected $user;

    public function __construct(User $user)
    {
        $this->user = $user;
    }

    public function find($user)
    {
        if($user instanceof $this->user) {
            return $user;
        }

        if($value = filter_var($token, FILTER_VALIDATE_INT)) {
            $key = 'id';
        } else {
            $key   = 'email';
            $value = $user;
        }

        return $this->user->where($key, 'LIKE', $value)->first();
    }

    public function flag($user)
    {
        if($user = $this->find($user)) {
            // Perform action on user
        }
    }

}

With this repository you can do:

$repo->flag(1);
$repo->flag(User::find(1));
$repo->flag('[email protected]');

@zenry // I'm not exactly sure what 'flag' does exactly..but...I prefer to add business logics into business objects. If we keep adding business object related methods into a repository, we are basically creating anemic objects. Not to mention that..I don't really understand why a repository should take a primitive data type for any save/updated related operations.

@zenri, Why are you using the second method call to the function with an User object? I believe that the logic of the repository pattern is to act as link between the Application Layer and the Data Layer for which you would be using an object from the Data Layer (User::find(1)) to obtain the same information.

Maybe I'm wrong.

Personally I use the first option, passing the instance of the User / object to act upon. I find it quite annoying to do ->find($userId) all over the repository than simply have the function accept a user-like model. It also makes it easy to have a user interface so your repository can be swapped out for another to change e.g. data storage format.

It's also because I tend to have the object already 'available' that I would like to update/flag/act upon in the controller, so simply passing it to the repository to do something with it makes sense, rather than the repository attempt to find the user again.