294156 - better normalization behaviour for calIDateTime

Status: RESOLVED WONTFIX

(Calendar :: Internal Components, defect)

Description

[Mike Shaver (:shaver emeritus)]

19:34 < vlad> shaver: so pav is trying to convince me to get rid of normalize()
19:34 < vlad> and to just always normalize() after every change
19:34 < vlad> but that would break if you ever want to create a duration

pav is right to do so, IMO; making normalization explicit makes callers think
about when/how to call it best (including unnecessary micro-optimizations that
clutter code) and make for clumsier manipulations.

I think we should autonormalize after every set (we can check if the cdt is in
need of normalization before actually making the icaltime call, if it profiles
as too expensive).

Durations should specify an isDuration flag on the cdt, analogous to isDate. 
This would suppress autonormalization, and would be required on anything that's
passed to addDuration/subtractDuration.  (I'm slightly, though not unassailably,
against a calIDuration type right now, though I think we might end up with one
as freebusy starts to surface in our apps.)

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

We can't normalize after any component (hour, minute, day, etc.) change; consider:

d = new CalDateTime();
d.hour = 26;
d.minute = 72;
d.day = 12;

you'd end up with something very much other than what you were expecting, since
the d.hour would end up with d.day to 1 more than what it was (zero?), and you'd
then overwrite it.  You could say that if you want to modify, you need to modify
in order, but that seems like a strange restriction.

I'll probably just add a dirty flag after each set, and then normalize before
any operation takes place using that datetime.

As for calIDuration, I'm tempted to create one -- it would simplify the
interfaces and usage a lot, and it's not like we're not already creating tons of
calIDateTimes when we really want a duration bit.  We'd end up with a
Subtract(in calIDateTime other)  on calIDateTime, along with AddDuration and
SubtractDuration.  calIDuration would have Add and Subtract.  I'm not too happy
with the name Subtract on calIDateTime, because Add doesn't make sense.. maybe
Duration()?  CalculateDuration()?  Something of that sort.

Comment 2

[Philipp Kewisch [:Fallen][📱📆]]

There is a subtractDate and the normalization hasn't done so much harm in the past. I think we should close this bug for now.