search

ocpsoft / prettytime

Social Style Date and Time Formatting for Java
http://ocpsoft.org/prettytime/
Apache License 2.0
1.29k stars 253 forks source link

Mutable methods are now protected #214

Closed gastaldi closed 3 years ago

gastaldi commented 3 years ago

In order to preserve the simplicity of the API and keep tests easy to write, I made the methods that change the internal state in PrettyTime package-protected.

This will require a major version bump.

Fixes #211

lincolnthree commented 3 years ago

Hey George, thanks for this :) I appreciate the PR.

I think the API needs to be exposed in some form, so this is an interesting idea. I guess that would work, but would require sub-classing - does it really solve the problem of immutability, or does it just push it downstream? You mentioned previously that the mutators should return new instances. I think that approach is probably more consistent with the concept of immutability.

Or did you have something else in mind? Does this actually solve the issue in Quarkus? I guess it prevents modification, so that would work. Yeah, just thinking out loud but I'm not sure making them protected is the right answer there. If anything, creating a wrapper class for Quarkus would make more sense than altering the existing API? Possibly a new builder facade?

Sorry again I've been so busy. Still trying to play catch-up with the launch.

gastaldi commented 3 years ago

Hey Lincoln!

I thought about introducing a Builder facade but at the same time I wanted to keep the API simple, which is what the existing constructors already provide.

A wrapper class in the Quarkus extension may make sense, but I believe this must be a core API design change.

I'll revisit the idea of exposing mutators but returning new instances, probably that is indeed a better solution