[threeten-dev] TemporalAdder and TemporalSubtractor

[Xueming Shen]

Stephen,

My guts suggest that these two still don't fit well into the rest of the 
"mini
temporal framework:-)" that you are busy building inside the temporal
package.

It has been a while since we spent lots of time trying to find the best 
solution
for the field/unit interfaces, lots of changes/evolution in this date/time +
field/unit interface group since then. The changes for the adjuster and 
query
appear reasonable and I agree the current approach provides good 
extensibility
and fits well into the coming lambda world. However I kinda feel we might
have better alternative for adder and subtractor interfaces.

Basically we need 2 types of the plus/minus support (on top of those fluent
style convenient plus/minus methods in those concrete date/time classes).

(1) Temporal + N TemporalUnit
(2) Temporal + Duration/Period

(1) has been well designed/implemented and very extensible, developer can
easily design/implement their own TemporalUnit and fit into the "framework"
smoothly, if they can not find the good match in existing ChronoUnit

The problem is the duration/period part. The current approach is to have two
powerful and "general purpose" interface adder and subtractor (like the 
adjuster)
to serve the purposes of (1) a common interface for 
duration/period/simpleperiod,
so those temporal classes don't have to have multiple individual methods 
for each
type (2) provide the extensibility for time duration/period.

My first concern with this approach is that we already have a general 
purpose/
powerful hook TemporalAdjuster in Temporal already, yes, it's "with" 
family, but
conceptually plus/minus is a kind of "adjust" operation as well. They take a
"temporal" and then turn/adjust it into something else with certain 
algorithm
(to tweak certain fields, or plus/minus something to certain fields). So 
do we
really need the second and third similar interface on a single/simple 
abstraction?
You can reasonably do anything to the "Temporal" with a "adjuster" and
make it a lambda expression, if desired.

Another concern is the mismatch of what we really need here and the rest
of the interface family (and naming as well). Developer will have to 
take two
new and "abstract" concepts here, the TemporalAdder and
TemporalSubstractor. Compared to their straightforward siblings a) fluent
style plus/minusDate/Time(...) and 2) plus/minus(N, Unit), which basically
says a certain point at time scale plus/minus a certain time duration, the
TemporalAdder/Subtractor appear to be strange, they are "something can
be added to a date/time/temporal". Given the nature of the plus/minus
operation on a time, why not just specify this "something" clearly, it's 
a "time
duration/period" (what else you can add and substract from a date/time?),
which is what we really need here. We need an abstraction/super interface
for Duration, Period and SimplePeriod, so Temporal.minus/plus can apply
on it. And something can also provide the extensibility if these three can
not meet what developers need in their specific scenario and have to
define something by themselves (but this something still have to be some
"time duration", right?)

And take a look at your "mini framework", we now have abstractions

Temporal and its read-only buddy TemporalAccessor, for date/time types
TemporalField, for those date/time fields
TemporalUnit, for those date/time units

the only piece missing is the "temporal" duration/period, we have
concrete classes of Duration, Period and SimplePeriod, but don't have a
"temporal" version of it. And it appears the Temporal.minus/plus() really
need such one to fit into its API. Instead of a pair of abstract "adder" and
"substractor", isn't a single TemporalPeriod a better alternative here?

I'm not sure what should be really in this super interface, maybe

abs(), isPositive(), doPlus()  and a doMinus() (arguably, if it's 
"directed",
maybe doMinus() is not necessary)

But this should not be an issue. Sure, we might miss the fancy functional
type with such an interface, but given the existing TemporalAdjuster, we
really don't need another similar weapon.

Personally I believe a TemporalPeriod is clear and simple, provides the
extensibility we are looking for and well fit into the rest of the APIs.

-Sherman