-
Notifications
You must be signed in to change notification settings - Fork 314
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 core classes final #34
Comments
Good point, they should be final. |
Such a change would contradict #22 as @rubenweerts made such changes to allow subclassing of |
I don't want to make the whole class final, but rather the implementation
details like methods such as addDelegate() ... But yeah, maybe a subclass
would like to override this, although I'm not sure in which use case
inheritance is needed at all (therefore making whole class final would be
ok) ...
Veyndan Stuart <[email protected]> schrieb am Di., 23. Mai 2017 um
04:15 Uhr:
… Such a change would contradict #22
<#22> as @rubenweerts
<https://github.com/rubenweerts> made such changes to allow subclassing
of AdapterDelegatesManager. I'm wary of the change made in that PR as is
evident by the most recent comment but I would be interested in the use
case of subclassing AdapterDelegatesManager as described by @rubenweerts
<https://github.com/rubenweerts>.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#34 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAjnri2_FmDMxIGsTKvjRFjxdl_1tKqSks5r8kFBgaJpZM4NCRP9>
.
|
If you set the implementation detail methods to final, wouldn't it just be preferable to set the whole class to final and favour composition over inheritance? |
I'd like to see a usecase where overwriting makes sense. I think that if you overwrite something right now you are speculating about the internal implementation. |
Is there any reason classes like
AdapterDelegatesManager
are not final? They really do not look like they are built for inheritance.The text was updated successfully, but these errors were encountered: