search

json-api-php / json-api

Implementation of JSON API in PHP
https://json-api-php.github.io/
MIT License
183 stars 18 forks source link

Is there a need for final keyword? #41

Closed ghost closed 7 years ago

ghost commented 7 years ago

I am implementing a presentation pattern that was to extend the ResourceObject to build a resource for specific parts of my application, apart from it uses the final class keyword.

Is it really a requirement to limit developer ingenuity by using final in the library?

Would it not be better to allow developers to make use of the classes available with inheritance?

ghost commented 7 years ago

I've submitted a PR #42

f3ath commented 7 years ago

Inheritance promotes tight coupling, final is a mean to avoid it by encouraging clients to use composition/aggregation instead of inheritance. Could you please elaborate on your particular use case? Can it be solved by composition?

ghost commented 7 years ago

I have an adapter class that makes a resource for a 'portfolio'. Using composition, I cannot pass this class to the Document as it doesn't implement the correct interface.

I was using inheritance to follow the Liskov substitution principle, as it is valid that my extension, or the base class, could be used interchangeably (granted the base class wouldn't set up the resource correctly, but hey).

I understand the argument about tight coupling, but as this is an adapter using interfaces in my system anyway, it's not a big issue in the context it's being used in.

I've jigged it to be a builder class for now.

I use final in code myself, but not in code used as packages for other systems as I find it too restrictive. It's my opinion though :)

f3ath commented 7 years ago

Thank you a lot for the feedback. I don't like ResourceObject in its current state, tbh. Mostly because of its mutability. As you see, the API is still at v0, so it's sort of unstable. For v1 I am considering making it immutable converting setters setFoo() into something like withFoo() which would produce another object. So it'll become a complete value object.

ghost commented 7 years ago

Cool. As this is now part of my API work at work, I may get involved in that as time allows. Sometimes mutability is a good thing, and sometimes it isn't. I'll get more up to speed with the project layout.

ghost commented 7 years ago

Closing as the PR was merged. Thanks! :)