[Kde-pim] KDE/kdepimlibs/kcal

[David Jarvie]

On Thursday 21 February 2008 16:57, Kevin Ottens wrote:
> SVN commit 777804 by ervin:
>
> Prokde35-s Item 61: Extended free/busy list support
> (kdepimlibs part)
>
> Merged revisions 744087-744110 via svnmerge from
> svn+ssh://ervin@svn.kde.org/home/kde/branches/kdepim/enterprise/kdepim
>
> ................
>   r744087 | vkrause | 2007-12-02 18:08:00 +0100 (dim, 02 déc 2007) | 2
> lines
>
>   Support for reading extended free/busy lists.
> ................
>   r744092 | vkrause | 2007-12-02 18:19:34 +0100 (dim, 02 déc 2007) | 2
> lines
>
>   Show extended free/busy information.
> ................
>   r744110 | vkrause | 2007-12-02 18:47:07 +0100 (dim, 02 déc 2007) | 9
> lines
>
>   Merged revisions 689908 via svnmerge from
>   https://vkrause@svn.kde.org/home/kde/branches/KDE/3.5/kdepim
>
>   ........
>     r689908 | bvirlet | 2007-07-19 15:23:54 +0200 (Thu, 19 Jul 2007) | 3
> lines
>
>     Remove these signal emissions already done in the subwidget.
>   ........
> ................
>
>
>
>  M  +9 -2      icalformat_p.cpp
>  M  +22 -0     period.cpp
>  M  +5 -0      period.h
>  M  +1 -0      tests/CMakeLists.txt
>  A             tests/testfb.cpp
> branches/kdepim/enterprise/kdepim/libkcal/tests/testfb.cpp#744087
[License: LGPL (v2+)]
>
>
> --- trunk/KDE/kdepimlibs/kcal/icalformat_p.cpp #777803:777804
> @@ -56,6 +56,7 @@
>  #include <ktzfiletimezone.h>
>  #include <ksystemtimezone.h>
>  #include <klocale.h>
> +#include <kcodecs.h>
>
>  #include <cstdlib>
>
> @@ -1177,13 +1178,19 @@
>      {
>        icalperiodtype icalperiod = icalproperty_get_freebusy( p );
KDateTime period_start = readICalUtcDateTime( p, icalperiod.start
> );
> +      Period period;
>        if ( !icaltime_is_null_time( icalperiod.end ) ) {
>          KDateTime period_end = readICalUtcDateTime( p, icalperiod.end );
> -        periods.append( Period( period_start, period_end ) );
> +        period = Period( period_start, period_end );
>        } else {
>          Duration duration ( readICalDuration( icalperiod.duration ) );
> -        periods.append( Period( period_start, duration ) );
> +        period = Period( period_start, duration );
>        }
> +      QByteArray param = icalproperty_get_parameter_as_string( p,
"X-SUMMARY" );
> +      period.setSummary( QString::fromUtf8( KCodecs::base64Decode(
param ) ) );
> +      param = icalproperty_get_parameter_as_string( p, "X-LOCATION" );
+      period.setLocation( QString::fromUtf8( KCodecs::base64Decode(
param ) ) );
> +      periods.append( period );
>        break;
>      }
>
> --- trunk/KDE/kdepimlibs/kcal/period.cpp #777803:777804
> @@ -51,6 +51,8 @@
>      KDateTime mEnd;      // period ending date/time
>      bool mHasDuration;   // does period have a duration?
>      bool mDailyDuration; // duration is defined as number of days, not
> seconds
> +    QString mSummary;
> +    QString mLocation;
>  };
>  //@endcond
>
> @@ -136,3 +138,23 @@
>    d->mEnd = d->mEnd.toTimeSpec( oldSpec );
>    d->mEnd.setTimeSpec( newSpec );
>  }
> +
> +QString Period::summary() const
> +{
> +  return d->mSummary;
> +}
> +
> +void Period::setSummary(const QString & summary)
> +{
> +  d->mSummary = summary;
> +}
> +
> +QString Period::location() const
> +{
> +  return d->mLocation;
> +}
> +
> +void Period::setLocation(const QString & location)
> +{
> +  d->mLocation = location;
> +}
> --- trunk/KDE/kdepimlibs/kcal/period.h #777803:777804
> @@ -187,6 +187,11 @@
>      void shiftTimes( const KDateTime::Spec &oldSpec,
>                       const KDateTime::Spec &newSpec );
>
> +    QString summary() const;
> +    void setSummary( const QString &summary );
> +    QString location() const;
> +    void setLocation( const QString &location );
> +
>    private:
>      //@cond PRIVATE
>      class Private;

Should these extra data members really be added to the Period class?
Period fulfils a basic function, to represent a time period, and I'm not
convinced that extra pseudo-random data which is required only by one
specific user (FreeBusy) should be added to it, especially since Period's
API is affected. Wouldn't it be possible to store this information in
another way within the FreeBusy instance, e.g. by creating a little class
like this:

class FBPeriod : public Period
{
  public:
    QString mSummary;
    QString mLocation;
};

This would avoid polluting the API for non-FreeBusy users of the class.

-- 
David Jarvie.
KAlarm author & maintainer.
http://www.astrojar.org.uk/kalarm


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