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

List:       kde-pim
Subject:    Re: [Kde-pim] [PATCH] Recurrences
From:       Reinhold Kainhofer <reinhold () kainhofer ! com>
Date:       2005-07-03 13:51:20
Message-ID: 200507031551.21762.reinhold () kainhofer ! com
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


Am Samstag, 2. Juli 2005 19:54 schrieb David Jarvie:
> On Friday 01 Jul 2005 00:39, Reinhold Kainhofer wrote:
> > Here is the patch for libkcal, which implements recurrences much better:
> > http://www.fam.tuwien.ac.at/~reinhold/KOrganizer/RecurrenceRule/
> >
> > 4) Changes in the recurrence rule don't trigger the event's update yet. I
> > don't want to pass the Incidence* pointer to the recurrence rule (I'd
> > like to keep the recurrence issues completely separated from the
> > incidence class)... If we get rid of the Incidence* pointer, we can
> > safely add copying for recurrences and then I suppose kalarm could get
> > rid of several hundred lines of code, which just implements copying of
> > recurrence data...
>
> Allowing copying of a Recurrence object would definitely be useful.

Yep.

> A couple of further comments about recurrence.h:
>
> Is it really sensible to remove the variants of methods such as
> setRecurStart(), durationTo(), etc., which take a 'const QDate&' parameter.
> This is a more logical way to do things for recurrences which float, and
> also for non-sub-daily recurrence types when you're really interested in
> the date rather than the time. For example, to find the number of
> recurrences up to and including a certain date, you would have to call
>       durationTo(QDateTime(date, QTime(23:59:59))
> instead of simply
>       durationTo(date).
> I can't see any real disadvantage of retaining these QDate versions, 

The API gets bloated by a factor 2. Besides, what's so bad about 
durationTo(QDateTime(date))? 
For floating events, the time part will be ignored, so it doesn't matter to 
what you set it (That's not yet working 100%).

If we also add methods that take QDate, we'll practically have to duplicate 
everything in the recurrence class, with one version that combines all QDates 
from the RecurrenceRules, and one that combines all QDateTimes from the 
RecurrenceRules. Is this really worth it?

> and 
> they do bring extra convenience for people using the class.

And lot of extra work for the people maintaining the class ;-)


> How about changing the parameter names in recurrence.h from "_r*" to
> something less obscure? I've always wondered why these names were used, 

Me too. 

> and 
> now seems a good time to, for example, change "short _rPos" to a simpler
> "short pos".

Yes, I overlooked this. Changed locally now.

> Well done for putting in the effort to tidy up recurrence handling - it
> certainly looks like an improvement.

Thanks.

Reinhold
-- 
------------------------------------------------------------------
Reinhold Kainhofer, Vienna, Austria
email: reinhold@kainhofer.com, http://reinhold.kainhofer.com/
 * Financial and Actuarial Mathematics, TU Wien, http://www.fam.tuwien.ac.at
 * K Desktop Environment, http://www.kde.org/, KOrganizer / KPilot maintainer

[Attachment #5 (application/pgp-signature)]

_______________________________________________
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]