rinvex / laravel-repositories

⚠️ [ABANDONED] Rinvex Repository is a simple, intuitive, and smart implementation of Active Repository with extremely flexible & granular caching system for Laravel, used to abstract the data layer, making applications more flexible to maintain.
https://rinvex.com
MIT License
666 stars 115 forks source link

Feature Request: Validations Rules Array #83

Closed sarfraznawaz2005 closed 8 years ago

sarfraznawaz2005 commented 8 years ago

It would be awesome if model validation rules array is added to package because then we won't need to use separate package (like Ardent) just for that.

Any thoughts about this ?

Thanks

Omranic commented 8 years ago

Well, I've few opinions here:

  1. Personally I don't like the concept of validating or caching data on the model level.
  2. For the validation process I think it's fair enough to use Laravel's default Form Requests as it filters & validate submited data before even reaching the controller itself, from the very early beginning.

Don't you agree, Or have different thoughts..?

sarfraznawaz2005 commented 8 years ago

I agree with you, having $rules array in model is especially inspired by Ruby on Rails apps, their models have validation rules as well as relationships as simple arrays which makes it easy to manage just at "one place". May be that's why Ardent is so popular. Secondly creating Form Requests, etc makes things more complex and time-consuming. Although it is different topic whether to put them in the model itself or not.

If we are to use this package, we have to go back and forth in writing repositories and also update laravel models for validations, relationships, etc, it would have been great if this was one place. I just felt the need of defining validation rules in repositry class which extends EloquentRepository something like:

protected $repositoryId = 'rinvex.repository.users';
protected $model = User::class;
protected $rules = [];

Having said that, I agree with you may be purpose of package is just focused on Repository Pattern (in which case it should also be framework-agnostic).

Thanks

Omranic commented 8 years ago

To be honest, I've thought about including validation features in this package before. I just still don't have a solid opinion on that yet. On a side note, I agree that maintaining form requests adds a more complexity to the project.

sarfraznawaz2005 commented 8 years ago

May be, let's keep this issue open and see what other users have to say ?

BTW thanks for awesome package :-)

sarfraznawaz2005 commented 8 years ago

My +1 for this feature :)

Omranic commented 8 years ago

Sounds great, and here's my thoughts as well, maybe someone will have vision of the ultimate validation solution 😄

Pros of including validation in this package

  1. Repositories are being used from anywhere, controllers, console commands, and other places, so it's not limited to the Http layer, accordingly I see it more logical to have validation rules on repository layer actually
  2. Placing validation rules in the repository layer will make thinks easier, simpler, more intuitive, and most importantly: centralized
  3. Centralizing validation rules, means omitting duplicate validation rules existing at Http layer, and Console layer, and anywhere else. It's good for logic & data consistency

Cons of including validation in this package

  1. I don't like the fact of re-inventing the wheel if not highly needed, and Laravel already have multiple awesome ways for validation, so I'd like to stick to the default features
  2. I still think that validating input data before touching the controller is much better than delegating it to the repository layer at a later stage of request lifecycle
  3. Adding such new features forces this package to have a diverged focus, adds overhead of more maintainability requirements, and most importantly leads to overloaded classes with too many aspects to take care of, and maybe violating SRP

These points IMHO weights each other, maybe someone have a better description & could convince us or reads things differently? Hopefully yes 😃 👍

Omranic commented 8 years ago

@ionut-tanasa @rsdev000 @evsign As heavy users of this package, what do you think of this proposal?

rjsworking commented 8 years ago

Hi. I don't have a opinion about performing validation inside the repo too. I've seen validation within the model, within controller and form requests. For medium (almost large) / large apps I prefer form request. It's centralized too. For small/medium (almost small) apps with less than half-dozen forms I would validate the data within the model. The only con I see is giving this package too many responsabilities.

Cheers

sarfraznawaz2005 commented 8 years ago

Hmm makes sense, cons seem to over-weigh the pros and you guys have raised valid points and package should not have too many responsibilities at the cost of possible technical debts.

ghost commented 8 years ago

In a "clean" way, respecting SRP and so on, the data needs to be validated before hitting the repository layer. The repository has to be dumb. It just takes the data and shoves it into a database.

Talking from my experience I prefer to use FormRequests. But not always are enough. I still use the good old Validator class. It depends on the use case.

And I think that the rules are useless on the repository if you use another package/or laravel itself to validate. At this point we could add the entire validation process inside the repository.

evsign commented 8 years ago

Agree with @ionut-tanasa, @rsdev000 and @Omranic ) I think form requests and maybe models are the best place to perform data validation.

Omranic commented 8 years ago

I'm glad we had this fruitful conversation, and the output we concluded 😄 Thanks everybody 👍 We keep this package focused on what it can do best, repositories & caching layer 😉