[Kde-pim] KCal::Calendar sortEvents

[Ron Goodheart]

On Sunday 05 October 2008 11:38:48 David Jarvie wrote:
> On Sat 4 October 2008 14:31:47 Allen Winter wrote:
> > On Friday 03 October 2008 03:26:59 Ron Goodheart wrote:
> > > Well, digging a little deeper...
> > > I think the sort code is actually okay, I think KDateTime is at fault.
> > > It comes down to a slight difference in how KDateTime and QDateTime
> > > handle date equality if one of the dates is a 'date only' type of date.
> > >
> > > QDateTime considers the 'date only' to be less than a date that has a
> > > time. KDateTime does not because it only compares the dates (unless the
> > > time zones are different then it changes the time if the first date is
> > > 'date only').
>
> QDateTime in Qt 4.4 doesn't have a concept of 'date only', so you can't
> compare it with KDateTime in this respect.
>
> > > The operator less-than code in KDateTime didn't look right so I ran a
> > > small test with the following results:
> > >
> > > dateOnly = "2008-10-02T00:00:00-07:00"
> > > datetime =  "2008-10-02T00:01:00-07:00"
> > > dateOnly isDateOnly? =  true
> > > datetime isDateOnly? =  false
> > > dateOnly <  datetime =  false
> > > dateOnly <= datetime =  true   <---
> > > dateOnly >  datetime =  false
> > > dateOnly >= datetime =  true   <---
> > > dateOnly == datetime =  false
> > > datetime <  dateOnly =  false
> > > datetime <= dateOnly =  true  <---
> > > datetime >  dateOnly =  false
> > > datetime >= dateOnly =  true  <---
> > > datetime == dateOnly =  false
> > >
> > > This is clearly incorrect!
> > >
> > > With the attached patch (with old code commented) I get:
> > > dateOnly = "2008-10-03T00:00:00-07:00"
> > > datetime =  "2008-10-03T00:01:00-07:00"
> > > dateOnly isDateOnly? =  true
> > > datetime isDateOnly? =  false
> > > dateOnly <  datetime =  true    <---
> > > dateOnly <= datetime =  true  <---
> > > dateOnly >  datetime =  false
> > > dateOnly >= datetime =  false
> > > dateOnly == datetime =  false
> > > datetime <  dateOnly =  false
> > > datetime <= dateOnly =  false
> > > datetime >  dateOnly =  true    <---
> > > datetime >= dateOnly =  true   <---
> > > datetime == dateOnly =  false
> > >
> > > I don't understand why the code makes a 'date only' end-of-day unless
> > > there was some kind of assumption of start-date and end-date? It does
> > > have an interesting side effect, but clearly not desirable.
>
> A date-only KDateTime value represents any time during the day from the
> beginning of the day to the end, so in effect represents the whole day in
> the context which you're discussing ( see the API documentation for
> KDateTime::operator<() ). This should explain the results you see.
>
> > > Are there automated tests for KDateTime that I should be checking?
> >
> > Ron,
> > There is a unit test for KDateTime in
> > kdelibs/kdecore/tests/kdatetimetest.c
> >
> > Make sure you run cmake with -DKDE4_BUILD_TESTS=ON
> > Then you should be able to go in the kdelibs 
builddir/kdecore/testsgreater-
> > and run ./kdatetimetest
> >
> > David Jarvie (who I'm sure is reading this) is the KDateTime guru.
>
> I just got back from holiday this evening and saw this.

Thanks, I read that, ran the unit tests and read the code and spent more time 
this weekend understanding what the comparison was doing.
Unfortunately for me I got your email afterward, oh well.

My understanding was clearly incorrect, not the comparison!

I didn't understand that the return value represented the result against that 
whole day in context from the docs. Given the way this works the 'date only' 
types should not be present in the sort. 

I'll need to go back to the original sorting statements to straighten out the 
printing, should be easy now with a clear understanding of why the sort was 
not returning the desired results. 

Thanks for the assistance and sorry for the confusion,
Ron

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/