Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the ArrayCollection extendable #80

Closed
wants to merge 1 commit into from
Closed

Conversation

Nek-
Copy link

@Nek- Nek- commented Apr 7, 2016

Hello,

I wanted to extends the ArrayCollection of doctrine to add some cool features. But unfortunately the elements are private so I can't implement new methods. I felt too bad (to copy/paste the class entire class) for a so little word like protected missing :) .

Of course you can stay on position that you don't want doctrine collection to be inheritable, but if you want that I suggest to set this class final... But I don't get why it would be.

@Nek-
Copy link
Author

Nek- commented Apr 7, 2016

By the way, I am extending the class to add removeItemThat(Closure) and things like that. Interested in a PR for that too ?

@Ocramius
Copy link
Member

Ocramius commented Apr 7, 2016

You shouldn't extend ArrayCollection: use the Collection interface instead.

@Nek-
Copy link
Author

Nek- commented Apr 7, 2016

@Ocramius what about setting it final ?

But well, let's say I did it... by copy-pasting the whole class :') .

@Ocramius
Copy link
Member

Ocramius commented Apr 8, 2016

@Ocramius what about setting it final ?

Some internal classes actually extend ArrayCollection to retain type compatibility with userland usage, but indeed, we should make it final in a major version (not now though)

@Nek-
Copy link
Author

Nek- commented Apr 12, 2016

@Ocramius I still don't get why I must copy/paste the ArrayCollection while I could inherit from it. Any good reason to share so I can understand this choice ?

@Ocramius
Copy link
Member

@Nek- mostly because the class is not designed for inheritance in first place. If we did that, we'd have to guarantee BC on all of its internals, which we don't want to deal with in first place. C&P is perfectly valid here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants