Timezone classes

[Shaheed]

On Thursday 15 September 2005 18:50, David Jarvie wrote:
> I have been looking at the kdelibs timezone classes with a view to using
> them in kdepim's libkcal for use with iCalendar files. I think that they
> need some amendments to make them usable for non-system timezones.

Thanks for the thoughtful comments.

> Although on the face of it the classes are well commented, in practice when
> one isn't familiar with them, it is difficult to work out what is the exact
> function and relationship of the different classes - I only started to get
> a grasp on it by studying the source code in some detail. Fuller class
> descriptions, and a better overview description, would be extremely useful.
> I could write something once the points below are addressed.
>
> There is a need to be able to construct multiple instances of KTimezones,
> since different timezone databases can contain the same zone names but with
> different definitions. Indeed the KTimezones class comments say that it
> "models *a* timezone database", which indicates slightly ambiguously that
> this is the intention. In that case, its constructor should not read in the
> system timezone database. The system timezones should only be included when
> specifically required - preferably in a KSystemTimezones class derived from
> KTimezones. KSystemTimezones should still allow the addition of extra
> private timezone entries so that a single database could be used, but
> splitting out KSystemTimezones would allow databases to be defined which
> conflict with the system one.

This is an excellent idea. I struggled with the notion of having a boolean 
passed into the constructor to suppress the defaults, but this is a much 
nicer idea.

> KTimezone::convert() and KTimezone::offset() are specific to the system
> timezone database, which ties the KTimezone class to being used for system
> timezones. There also needs to be provision for custom timezones. As with
> the KTimezones class, this would be better implemented by a KSystemTimezone
> class to hold system timezones, inherited from a virtual base class
> KTimezone. There could also perhaps be another kdelibs class derived from
> the KTimezone base class to form the base for private timezone classes. The
> common need for such classes would be to hold their own details of time
> offsets and changes.

Also makes sense to me.

> KTimezones::local() doesn't allow for the possibility of /etc/localtime
> being a link to a file in /use/share/zoneinfo/ (or wherever). This is quite
> a common configuration, e.g. in Debian. The method also reads the files in
> /use/share/zoneinfo/ *every* time it is called, which seems an unnecessary
> overhead. Could the files' checksums be cached, and the files only be
> reread if no match is found (which might indicate that the timezone
> database had changed)?

I commented on the link thing elsewhere. As for caching, it is deliberate, 
otherwise the user could change the timezone and you would not see it. If you 
want it cached, you can always do so!

> KTimezoneDetails class methods need comments indicating what the parameters
> represent. It isn't obvious from the parameter names without investigating
> the details of the zoneinfo file structure. Presumably zoneinfo is
> documented somewhere - it might be worth putting in a cross reference to it
> in the comments.

There is indeed a man page on zoneinfo. I will look at improving the comments 
here.

I won't have a chance to look at this for a while, if you want to have a hack 
in the meantime, be my guest. There is a small testsuite in the tests 
directory.

Thanks, Shaheed