[Kde-pim] Replacement for KDateTime

[John Layt]

On 9 August 2015 at 21:06, Sergio Martins <iamsergio at gmail.com> wrote:
>
> On Sunday, August 09, 2015 20:51:53 John Layt wrote:
> > OK, next bout of insomnia I might try a few experiments.

Right, made a start, mostly expunged the dateOnly stuff without a
public CalDateTime class with no ill effects.

Now I've hit the issue of what to do with KDateTime::Spec. In
KDateTime the details of the time spec (offset, tz, utc, etc) are kept
in a separate class KDateTime::Spec which gets passed around in the
KCalCore api for things like setting the default spec to use in a
calendar, or shifting all incidences into a different spec. QDateTime
doesn't have a separate Spec class, it's all just embedded in
QDateTime itself.

This creates an issue with the public api of methods like:

    Calendar(const KDateTime::Spec &timeSpec);
    void setTimeSpec(const KDateTime::Spec &timeSpec);
    KDateTime::Spec timeSpec() const;
    void shiftTimes(const KDateTime::Spec &oldSpec, const
KDateTime::Spec &newSpec);

Calls to these methods usually look like this

    cal.setTimeSpec(KDateTime::Spec(KTimeZone(tz)));
    cal.setTimeSpec(KDateTime::Spec(KDateTime::OffsetFromUtc, 3600));
    cal.setTimeSpec(kdt.timeSpec());
    cal.setTimeSpec(m_spec);

I see 3 choices for handling this.

1) Replace KDateTime::Spec in the api with QDateTime and document that
the actual datetime value will be ignored. Quick and easy to achieve,
but possibly confusing. e.g.:

    Calendar(const QDateTime &timeSpec);
    void setTimeSpec(const QDateTime &timeSpec);
    QDateTime timeSpec() const;
    void shiftTimes(const QDateTime &oldSpec, const QDateTime &newSpec);

    cal.setTimeSpec(QDateTime(QDate(), QTime(), QTimeZone(tz)));
    cal.setTimeSpec(QDateTime(QDate(), QTime(), Qt::OffsetFromUtc, 3600));
    cal.setTimeSpec(qdt);
    cal.setTimeSpec(m_spec);

2) Replace the api with each of the possible specs that can be used.
Fairly easy to achieve, but bloats the api and makes the calling code
check for itself which to call (but it often does that anyway when
creating a new Spec). The shiftTime api is particularly bad in this
option, e.g.

    Calendar(const Qt::TimeSpec &timeSpec, offsetFromUtc=0);
    Calendar(const QTimeZone::Spec &timeZone);
    void setTimeSpec(const Qt:TimeSpec &timeSpec, offsetFromUtc=0);
    void setTimeSpec(const QTimeZone::Spec &timeZone);
    Qt::TimeSpec timeSpec() const;
    int offsetFromUtc() const;
    QTimeZone timeZone() const;
    void shiftTimes(const Qt::TimeSpec &oldTimeSpec, oldOffsetFromUtc,
                      const Qt::TimeSpec &newTimeSpec, newOffsetFromUtc);
    void shiftTimes(const Qt::TimeSpec &oldTimeSpec, oldOffsetFromUtc,
                      const QTimeZone::Spec &newTimeZone);
    void shiftTimes(const QTimeZone::Spec &oldTimeZone,
                      const Qt::TimeSpec &newTimeSpec, newOffsetFromUtc);

    cal.setTimeSpec(QTimeZone(tz));
    cal.setTimeSpec((Qt::OffsetFromUtc, 3600);
    if (kdt.timeSpec() == Qt::TimeZone)
        cal.setTimeSpec(qdt.timeZone());
    else
        cal.setTimeSpec(qdt.timeSpec(), qdt.offsetFromUtc());
    if (kdt.timeSpec() == Qt::TimeZone)
        cal.setTimeSpec(m_timeZone);
    else
        cal.setTimeSpec(m_imeSpec, m_offsetFromUtc);

3) Create a new class CalTimeSpec based on KDateTime::Spec but using
QDateTime stuff instead, e.g.

    class CalTimeSpec {
        CalTimeSpec(const QDateTime &timeSpec);
        CalTimeSpec(const Qt::TimeSpec &timeSpec, offsetFromUtc=0);
        CalTimeSpec(const QTimeZone::Spec &timeZone);

        Qt::TimeSpec timeSpec() const;
        int offsetFromUtc() const;
        QTimeZone timeZone() const;
   }

    cal.setTimeSpec(CalTimeSpec(QTimeZone(tz)));
    cal.setTimeSpec(CalTimeSpec(Qt::OffsetFromUtc, 3600));
    cal.setTimeSpec(CalTimeSpec(kdt));

Any thoughts on preferred method, or some hybrid of some or all of
them? I suspect CalTimeSpec is the cleanest approach, but it makes
some calls unnecessarily verbose, so perhaps supplement it with
convenience methods that take the base spec components or a QDateTime
and creates CalTimeSpec in the background?

> We are releasing the applications (branch Applications/15.08, already frozen)
> but the libraries are just a private implementation detail at this time,
> there's no kf5 kdepim libraries release, no ABI or source compat promisses.

Ummm, that's an interesting approach. So what version number will the
libraries carry? I sure hope the communication is clear these are not
to be used externally!

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