Timezone classes

[David Jarvie]

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.

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.

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.

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)?

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.

-- 
David Jarvie.
KAlarm author and maintainer.
http://www.astrojar.org.uk/linux/kalarm.html