[Development] Calendar Systems proposal

[Edward Welbourne]

Thiago Macieira (7 August 2017 06:14)
> ... there are bigger problems with the implementation, starting with
> QAbstractCalendar having a static protected QMap member.

That's my fault.  We're going to need some way for QML/V4 to get at
calendars; and I want to ensure our design leaves scope for client code
to provide a calendar of its choice - e.g. so that a developer whose
target market uses a relatively obscure calendar can support them with a
minimum of pain, without Qt itself needing to contain code for every
calendar that ever has been or will be invented.  (Conversely, of
course, it's also worth making it easy to configure which, of the
calendar implementations present in Qt, actually get compiled into any
given build.)  Each calendar implementation needs a way to make itself
visible to QML/V4.

My naive idea for how to solve that was to let each calendar
implementation register a factory for its objects under a name
representing its calendar.  Then QML/V4 (and, for that matter, C++ APIs)
can look up a calendar by name and get a calendar object to use.
Indeed, we can even consult the map for a list of supported calendars,
to populate a UI's drop-down to select which to use.

Please suggest a better solution (and explain the problem with the
present solution), either here or in the review.

> The big problem is how you've implemented the new API in QDateTime and
> QLocale. There's code duplication that cannot be there in QLocale,

That's probably best addressed by you commenting on the review; I'm not
sure what duplication you're referring to ("cannot be there" is strong
language), although I do know about dateTimeToString().  There are a few
places I expect to find myself doing clean-up in the aftermath of
getting this all in, but I don't mind doing some of it before-hand.

Note, also, that this moves calendar-related data out of QLocale's
CLDR-derived data blob into calendar-specific data blobs - a step in the
general direction of making QLocale less monolithic.

> but the way you've removed the duplication in QDateTime also needs
> changing for performance reasons.
>
> int QDate::year() const
> {
>     return year(QGregorianCalendar());
> }
>
> This creates a polymorphic object and makes a call that ends up delegating to
> it in
>
>             if (cal.julianDayToDate(jd, y, m, d))

Please elaborate: why is this a problem ?  The Gregorian arithmetic
naturally belongs in a calendar implementation.  The date-time code
should naturally call it, rather than duplicating it in static functions
of its own (which have, naturally, been moved to become its methods).

> The commit also includes changes that look like unrelated clean-ups and will
> need to be split into different commits.

Please point these out on the review.  Some of them might not be as
unrelated as you think - I did a fair bit of pulling separable changes
out already, rebasing Soroush's work onto the results.  I may well have
missed some, but there were bits that couldn't be separated.

> It's at this point lacking documentation

Indeed, there remains work to be done.  On the other hand, deciding what
shape the API should be is worth doing before taking pains over
documenting every detail - that would all change if we decide we need to
change the API.

> and there are a couple of coding style mistakes.

Please note in Gerrit.

	Eddy.