[Kde-pim] Questionable change to KCal::Period class

Allen Winter winter at kde.org
Sat Feb 23 13:44:57 GMT 2008


On Saturday 23 February 2008 05:55:41 Till Adam wrote:
> On Saturday 23 February 2008 02:26:49 David Jarvie wrote:
> > I'm repeating this posting with a different subject line, and
> > top-posting, to elicit some response.
> >
> > The commit below alters the kcal Period class in what I think is an
> > undesirable way. It adds summary and location members to the class, which
> > I don't think belong there. What is the relevance of these members to a
> > class which encapsulates a time period? Their values come from custom
> > properties in iCalendar, and AFAICS only one user of the Period class
> > (FreeBusy) will ever use them. Instead of changing the Period class,
> > FreeBusy should store this data itself, perhaps by using a little class
> > something along the lines of
> >
> > class FBPeriod : public Period
> > {
> >   public:
> >     QString mSummary;
> >     QString mLocation;
> > };
> >
> > It may be necessary to juggle things or add new methods to keep
> > FreeBusy's API BC, but I think this is preferable to polluting the Period
> > class for non-FreeBusy users. It's a bit reminiscent of the Pilot-special
> > stuff which was once in icalformatimpl, which was eventually moved to
> > where it belongs, in KPilot itself.
>
> I agree with you in principle, David. As this is the result of a merged
> enterprise feature, I'll see about getting it rectified.
>
> Thanks for the review,
>
I also agree with David, upon further review.
For example, the Period comparison operators look silly now.

So I'd also like to see a better solution.

-Allen

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



More information about the kde-pim mailing list