Use constructor property promotion for ContactBundle classes #7435
Conversation
mamazu
force-pushed
the
cpp_contact
branch
2 times, most recently
from
ff51a3a to
5c9bb57
Compare
| @@ -81,31 +66,22 @@ class ContactManager extends AbstractContactManager implements DataProviderRepos | |||
| */ | |||
| protected $userRepository; | |||
There was a problem hiding this comment.
Sadly we can't promote protected properties as this would enforce the types on the properties (and be a bc break). I don't think they should have been protected in the first place though.
There was a problem hiding this comment.
We could still promote them just with phpdoc instead of hardcoded type.
There was a problem hiding this comment.
This would remove the typehint for the constructor but once all types and the doc comments are gone, this should be a good solution.
There was a problem hiding this comment.
Alternatively, I don't think this is a huge bc break. Because if you put any other classes into that property the class is going to crash anyways. So I think promoting that and keeping the typehint should be fine.
There was a problem hiding this comment.
We should keep a eye if everywhere the Interfaces if exists of the service was used. Else I think its not a problem as then service decoration and overwrite by inheritance works.
There was a problem hiding this comment.
Will do, it's probably not the last time that I'll go over classes. And specific types should be replaced with interfaces whereever possible anyways.
mamazu
changed the title
| $listBuilder->setIdField($fieldDescriptors['id']); | ||
| $listBuilder->where($fieldDescriptors['account'], $id); | ||
| $listBuilder->sort($fieldDescriptors['lastName'], $listBuilder::SORTORDER_ASC); | ||
| if ($listBuilder instanceof DoctrineListBuilder) { |
There was a problem hiding this comment.
We should avoid such changes in this kind of refactors. Also in this case we should just assert that $listBuilder is DoctrineListBuilder so \assert($listBuilder instanceof DoctrineListBuilder); after $this->listBuilderFactory->create would be better. but also @var is in this case enough as the DoctrineListBuilderFactoryInterface always returns a DoctrineListBuilder and the controller should so not care and would require to fix it with generics in the list builder interfaces classes itself. But as said that kind of tasks should be in seperate pull request to keep things concentrated on one single refactors.
alexander-schranz
changed the title
alexander-schranz
force-pushed
the
cpp_contact
branch
from
8476c0e to
b40eb08
Compare
|
@mamazu Thank you! |
What's in this PR?
Using constructor property promotion for the Contact Bundle.
Why?
Less code = Better