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

Allow class overriding #149

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow class overriding #149

wants to merge 1 commit into from

Conversation

avargas
Copy link

@avargas avargas commented Apr 15, 2021

I wanted to experiment with a subtle behaviour change and stumbled upon a limitation that I could easily fix by extending my own Router. In theory it should be fine, but some methods and instance variables are set to private, when they could be set to protected and allow overriding.

I wanted to experiment with a subtle behaviour change and stumbled upon a limitation that I could easily fix by extending my own Router. In theory it should be fine, but some methods and instance variables are set to `private`, when they could be set to `protected` and allow overriding.
@bramus
Copy link
Owner

bramus commented May 12, 2021

No too keen on making all of these protected, as they're internal things that can break things when overridden wrongly.
What exactly do you want to achieve?

@avargas
Copy link
Author

avargas commented May 18, 2021

Well, I wanted to change the behaviour of handle() to do something custom I needed.

While your point is true, my suggestion is that some people might be keen to tweaking behaviour and having fields and methods as private stops this behaviour. I think they should be protected to allow anyone from overriding behaviour, that's the point of it being open source, I think.

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

Successfully merging this pull request may close these issues.

2 participants