[Libreoffice] [REVIEW][3-4] date change regression fdo#44385

[Andrew Douglas Pitonyak]

On 01/20/2012 06:32 AM, Noel Power wrote:
> Hi Andrew,
>
>> My primary concerns
>>
>> 1. Change in behavior that may break existing code.
> the intent of the patch is to restore the previous behaviour ( by 
> previous behaviour I mean before integration of the feature that 
> erroneously removed the 'special' processing for '+' for dates ). 
> Additionally the intent of the patch is to make the behaviour for '-' 
> similar for'+' e.g. if Date + 1 gives a Date then surely Date - 1 
> should additionally give back at Date. Hopefully that makes sense :-)

So the intent was not original behavior, but the behavior that was 
intended the first time but then left commented out. You then did one 
better by allowing for subtraction. Now I know what you mean by "old 
behavior". good.

>>
>> 2. Previous behavior was never flagged as an issue, and may arguably 
>> be considered logical to some. for example, storing a "delta" in the 
>> native type.
> I don't think we should be afraid to change present behaviour if we 
> think it is wrong and especially if the impact is probably going to be 
> very minor ( as I think is the case here ). I would bet that the 
> behaviour we have at the moment is an incomplete attempt to behave in 
> a compatible way with vba. Where we can, I think if the cost is low we 
> should try to be compatible ( should actually look at what vba does 
> [*]). But... I take it that you are unhappy with changing the 
> treatment of '-' ? and wouldn't be in favour of the change right? ( 
> funny somehow I thought in the previous mail thread that you were in 
> favour of even letting the Date type be set for all those numeric 
> operations, sorry I misunderstood you )

Oh no, I think that we need it on minus as well. I did state that your 
change covered the majority case. My concern is that it may break 
existing macros, which is how I found the bug in the first place. I 
believe that your change will allow that particular macro to function 
correctly. The macros that will break are macros that assume (date +/- 
date) return date or that other operations such as * return date. 
Logically, however, I do agree that (date - date) makes little sense as 
a date and I have trouble envisioning the meaning of date * date.

I believe that your fix will allow date - date + date to return a date, 
so in my brain, I expect it to work as I expect.

>>
>> 3. The rules are more complicated now and require complex examples to 
>> explain; for example, time_1 + time_2 + time_3 returns a date but 
>> time_1 + time_2 does not. We cannot simply state that math done with 
>> dates or including a date returns a date.
> yeah that doesn't sound right :-/  should remain a Date I would think

In a quick test under OOo, I did not test on LibO, I noticed that the 
Time() function returned a different type than Now(). Oh, I see that 
Time() returns a string. That explains a lot. A "real" time is (as I had 
original thought) expressed as a Date that has a 0 integer portion.

I do believe, however, that all the original arguments for a Date are 
the same for time. Logically, using time, 13:00 + 13:00 means nothing 
that makes sense to me, so you still covered the majority case unless 
someone chose to store elapsed time in a date object.

>
> [*] table of behaviour in vba, pre the bug being introduced and post 
> patch
>
>                                mso    old behaviour  present
>                             --------------------------------
> date + num                    Date      Date          Date
> date - num                    Date      Double        Date
> date * num                    Double    Double        Double
> date / num                    Double    Double        Double
> date + date                   Date      Double        Double [1]
> date - date                   Double    Double        Double
> date * date                   Double    Double        Double
>
> [1] the date + date is clearly wrong (fixing that would address '3.' 
> above. )
>
> personally I am all for fixing this to be compatible

The only thing with which I disagree is that I would have addition and 
subtraction return the same types. So, I would say that if date + date 
returns a date (as your chart shows for MSO) then I would have date - 
date return a date as well. If not, then I would leave your original 
change as is so that they are consistent for addition and subtraction.

What type is "-date"? I see that never returned a date so (date + 
(-date)) will return a date.


-- 
Andrew Pitonyak
My Macro Document: http://www.pitonyak.org/AndrewMacro.odt
Info:  http://www.pitonyak.org/oo.php