Skip to content

Remove final keywords #198

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

Closed
antoinelame opened this issue Jan 16, 2024 · 5 comments
Closed

Remove final keywords #198

antoinelame opened this issue Jan 16, 2024 · 5 comments

Comments

@antoinelame
Copy link
Contributor

Would you be open to removing the final keywords from the Type and PseudoType classes to enable people to extend them in their projects? Alternatively, could they be replaced with @internal annotations?

I would be happy to submit this PR.

@jaapio
Copy link
Member

jaapio commented Jan 16, 2024

I would like to understand your use case, and if we should add some missing parts in this library for others who are facing equal challenges like yours. Maybe I have missed something that should have been implemented. We did open some of the classes in the past to allow people to make their own pseudo types. So there are use cases that are intended to extend the existing classes. And we provide a way to overwrite existing keywords. On the other hand we know that we should be very carefully with introducing new keywords in this library, as keywords will block you from creating classes with that name as many frameworks as using this library to add dynamic behavior.

Just opening up all classes to be extended doesn't feel good. We closed them with intentions not to allow extension. As it will make sure that nobody is using the library in a different way than what we intended with it. This allows us break the internal stuff without warning. Like the switch to a different parser strategy which we did some year ago. So from a maintenance point of view this is a benefit.

I hope you can explain your use case, so we can find a solution for it.

@antoinelame
Copy link
Contributor Author

While an IntergerValue is already provided by the library, I created a new one in my project because I needed it to be a subclass of Interger, which is not the case with the provided one. Same with String / StringValue. However, Float_ is final so I couldn't do the same.

@jaapio
Copy link
Member

jaapio commented Jan 17, 2024

Removing it from Float makes sense, please create a pr for that, I would merge that

@jaapio
Copy link
Member

jaapio commented Jan 19, 2024

@antoinelame is this solved for you know or is there more you would need?

@antoinelame
Copy link
Contributor Author

This is solved, thank you.

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

No branches or pull requests

2 participants