Happyr / Doctrine-Specification

This library gives you a new way for writing queries. Using the Specification pattern you will get small Specification classes that are highly reusable.
MIT License
446 stars 41 forks source link

Use defaultRepositoryClassName instead of the RepositoryFactory #297

Closed thexpand closed 2 years ago

thexpand commented 3 years ago

I find it more convenient to use the defaultRepositoryClassName of the Doctrine configuration. Your RepositoryFactory seems to be a bit harsh and doesn't take into account if I currently have entities with their own custom repositories, which makes it hard to refactor a large codebase gradually. I ended up setting EntitySpecificationRepository as the default repository class name. This way I'm still using the default repository factory that is provided by Doctrine, which will create the specification repository as a fallback. When I want to, I can use the EntitySpecificationRepositoryTrait trait in the current repositories and from there I can gradually refactor the methods into specifications.

Do you think that this should be included somewhere in the documentation, because now it only suggests to use the factory?

peter-gribanov commented 3 years ago

I ended up setting EntitySpecificationRepository as the default repository class name. This way I'm still using the default repository factory that is provided by Doctrine, which will create the specification repository as a fallback. When I want to, I can use the EntitySpecificationRepositoryTrait trait in the current repositories and from there I can gradually refactor the methods into specifications.

This is what the documentation for Symfony and Laravel suggests to do. Perhaps you are looking at the documentation for Zend 1 or Zend 2? For Zend, the documentation does suggest using a factory.

peter-gribanov commented 3 years ago

I think we can make another factory that allows custom repositories. But i'm not sure if that makes sense since it will act as a Doctrine repository factory. I would say that using the default_repository_class option is the preferred solution.

thexpand commented 2 years ago

I see now that there has been a PR that is already merged. Shall I close this issue now?

peter-gribanov commented 2 years ago

Yes. Of course.