Best if Repositories always return ->toArray() ?

Published 4 months ago by miwal

Am I right in saying that to implement a repository effectively, I must not return any active records (models).

Reason is, a (perverse) client could do this:

// $lessons is the name of the injected repository

$lesson = $lessons->get($slug);

$any_other_mad_query = $lesson::find('some other lesson');
// this last call circumvents the repository, so if the data layer had been derouted, say, to a mock, or some other implementation of the repository than the database which the model connects to, this would break.

The above code circumvents the repository and breaks separation of concerns. It's what you risk if your repository returns a functioning active record.

To prevent this, repositories should always call ->toArray().

toArray() will unnest even the eloquent collection of models (active records). Here's a summary of the options (I checked):

  • Lesson::first() gives a single model, not a collection.
  • Lesson::all() gives an eloquent collection (extends base collection) containing models.
  • Lesson::first()->toArray() gives a flat array.
  • Lesson::all()->toArray() gives an array of arrays (both collection and models inside it are unpacked to arrays)

Taylor calls ->toArray() when returning from a repository at the start of his book.

I just wanted to get my head around the concept before attempting to implement it, the options available, what to avoid and why.

Comments / clarifications / discussion greatly appreciated.

skliche
skliche
4 months ago (149,590 XP)

Let's assume you want to modify the models and persist those changes. If your repository returns arrays only then you'd have to re-create the models or perform update queries instead of using the already existing functionality.

Furthermore the code that "consumes" the data returned by the repository would loose all of the awesome functionality that both the collection and the model provide.

If you replace the current implementation with a different implementation then that implementation should adhere to the same contract. If a method returns a collection of models then a different implementation should return a collection of models as well.

As always it depends on what you are trying to achieve. But in general I'd say return collections of models or single model instances.

miwal
miwal
4 months ago (44,170 XP)

Thanks @skliche. I tried sketching some client code and see what you mean.

A problem, though, if I do use any of those eloquent calls on a returned model, is it appears the principal benefit on which repositories are being sold, viz. swappability (e.g. to add a caching layer, which is what I had in mind to do with it at this point), will be lost (!?)

(since any call on a returned model will bypass the repository).

A problem... :) Is this a problem with all use of the repository pattern in laravel?

At this point I'd like to see some example repository contracts to compare my needs against, do some drafting and see how it could stack up in reality for my case.

Wonder if it would be useful at this point to try reading the laravel source. Not sure where to start with that though :) I tried a few times and didn't get much leverage on it. Wonder what some of the most accessible parts to start reading first could be? Cheers.

skliche
skliche
4 months ago (149,590 XP)

it appears the principal benefit on which repositories are being sold ... will be lost

I don't think so. You could still extend or decorate the repository and add the caching logic there. E.g. Cache::remember calling the repositories method. If you think it might be necessary to swap out Eloquent at some point, ok. But why and when would you want to do that? (not saying it's wrong, I just can't come up with a use case)

Have a look at query objects if one of your objectives is not to pollute the models or even the controllers with large, complicated queries.

since any call on a returned model will bypass the repository

But is it necessary to go through the repository? Is the repository the right place to act on a model instance? Is the repository responsible for iterating the results, for filtering or mapping them? The answer to these questions might be both yes and no, depending on the use case or personal preference.

Have a look at https://laracasts.com/lessons/the-repository-trap-and-other-ramblings and https://laracasts.com/lessons/repositories-simplified The comments there might address some of your concerns.

In my opinion it all depends on what exactly you are trying to achieve right now (not in a possible future) and what option addresses your needs (like caching, test, ...). Your needs will change, requirements will change, your opinions and preference will change.

Build a repository that provides both model instances and stdclass objects/arrays. See how it changes the rest of the code. See how it affects working with relationships. See what interface you prefer to use.

miwal
miwal
4 months ago (44,170 XP)

it all depends on what exactly you are trying to achieve right now

Build a repository that provides both model instances and stdclass objects/arrays. See how it changes the rest of the code

My action now precisely :)

biishmar

From repository function use the parameter array as false as default by this u will get the model instance if u want array change that parameter to true, then u get array of data.

Please sign in or create an account to participate in this conversation.