search

brick / date-time

Date and time library for PHP
MIT License
326 stars 30 forks source link

Consider making value classes final #8

Closed joshdifabio closed 4 years ago

joshdifabio commented 5 years ago

The value classes should really be final.

BenMorel commented 5 years ago

Good point, I usually make my classes final unless they are explicitly meant to be extended, not sure why I did not do this at the time.

BenMorel commented 5 years ago

Done in 99c1f65d542360dc8ef49d07b962afb62f14ef18, will be released as 0.2.0 once #10 is complete.

natebrunette commented 5 years ago

I was going to suggest adding better support for extending classes like ZonedDateTime. I'd like to be able to extend it, and it would be helpful if static was used instead of the class name. I want to be able to augment the class with app-specific methods, but currently can't without copying most of it.

joshdifabio commented 5 years ago

These are value objects -- you shouldn't really be extending them.

Testing equality is fundamental on VOs and being able to subclass a VO complicates or breaks that feature.

BenMorel commented 5 years ago

I'm on the same page as @joshdifabio here; you should favour composition over inheritance, i.e. create your class from scratch, wrapping an instance of ZonedDateTime and delegating parts of the work to it. Yes, some methods will just be proxies to the underlying object methods, but you'll expose only the methods you need, and are free to change their behaviour as you wish (like making them return an instance of your class).

I'm usually against extending classes from a library, unless they've been specifically designed for this purpose. In addition to not being good practice, extendable classes add to the maintenance burden by adding backwards compatibility promises to internal code.

Maybe if you tell us a bit more about your use case, we can help you refactor it towards composition. What are you extending ZonedDateTime() for?

BenMorel commented 4 years ago

Done in 9625e8408b7136b2447a61e7a3d046caede0f573 and released in 0.2.0.