[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-pim
Subject:    Re: [Kde-pim] [PATCH] Recurrences
From:       David Jarvie <lists () astrojar ! org ! uk>
Date:       2005-07-18 21:09:22
Message-ID: 200507182109.22891.lists () astrojar ! org ! uk
[Download RAW message or body]

On Friday 15 Jul 2005 18:08, Reinhold Kainhofer wrote:
> I now created a new patch for the recurrences:
> http://www.fam.tuwien.ac.at/~reinhold/KOrganizer/RecurrenceRule/Recurrences
>_2005-07-15.patch
>
> Am Samstag, 2. Juli 2005 10:38 schrieb David Jarvie:
> > Recurrence::endDate() - this returns a date-time value, so its name isn't
> > very intuitive, suggesting as it does that it returns a QDate.
>
> Okay, for the Recurrence class I added QDate methods and use endDate() and
> endDateTime() (same for the start date, which I renamed from recurStart to
> startDate[Time]). I also re-added the QDate-version of durationTo.
>
> But I won't add a getNextDate or getPreviousDate. I think getNextDateTime
> and getPreviousDateTime suffices there.

That's fine by me. Here are some detailed comments on the Recurrence class:

endDateTime(), getNextDateTime() and getPreviousDateTime() assume that the 
time for mRDates entries is 00:00:00. I'm not convinced that this is wise - I 
would have thought that the DTSTART time would be a better assumption.
Unfortunately, RFC2445 doesn't seem to give any guidance.

endDateTime(), getNextDateTime() and getPreviousDateTime(): Rather than 
accumulating all the date-times and then doing a sort in order to find out 
the latest/earliest, it would be more efficient to simply compare each one as
it is calculated with the last, and note the latest of the two.

The following methods only set or return values for the default RRULE, rather 
than for all recurrence components combined.  This is not what one would 
expect, and not what the comments in the header file say.
    setEndDate()
    setEndDateTime()
    duration()
    durationTo()
    setDuration()

unsetRecurs() only removes RRULES - one would expect, and the header comments 
indicate, that it would clear all the other components as well.

The header comments for the following methods should make clear that they only 
apply to the default RRULE:
    frequency()
    setFrequency()
    days()
    monthDays()
    monthPositions()
    yearDays()
    yearDates()
    yearMonths()
    yearPositions()
    addMonthlyPos()
    addMonthlyDate()
    addYearlyDay()
    addYearlyDate()
    addYearlyPos()
    addYearlyMonth()

The header comments for setMinutely(), setHourly(), etc. should state that 
this replaces any existing RRULEs but leaves all other components unchanged.

setExDates() and setExDateTimes() need to sort mExDates/mExDateTimes, since 
operator==() expects them to be in a predictable order, and addExDate() and 
addExDateTime() sort them.

setRDates() and setRDateTimes() need to sort mRDates/mRDateTimes, since 
endDateTime(), getNextDateTime() and getPreviousDateTime() expect them to be 
sorted.

-- 
David Jarvie.
KAlarm author and maintainer.
http://www.astrojar.org.uk/linux/kalarm.html
_______________________________________________
kde-pim mailing list
kde-pim@kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
kde-pim home page at http://pim.kde.org/

[prev in list] [next in list] [prev in thread] [next in thread]