On Friday 30 September 2005 10:55, David Jarvie wrote:

> AFAICS your revisions consist simply of splitting out the system time zone
> stuff from KTimezones into a new class KSystemTimezones. They don't address
> the issue that the other classes as they stand are still not suitable as
> base classes for implementing access to non-system time zone sources.
> KTimezone and KTimezoneSource are both tied to the system time zones and
> data formats - KTimezoneSource reads data in TZFILE (zoneinfo) format only,
> while KTimezone methods do calculations using tzset() which will only work
> if the time zone names are known to the system and if the definitions of
> those time zones are the same as the system definitions.
>
> What is needed is a set of classes which can be used for subclassing for
> use with other sources and formats of time zone information, such as
> iCalendar. My proposals try to address this need, by providing a completely
> general set of base classes which should work (by subclassing) no matter
> what the format of the data, and no matter what the access method (data
> file, text string, binary data, via external library calls, ...).

I accept that I started from a design that had no concept of (or concern for!)  
custom timezones, and then probably made a half-baked attempt to implement 
them based on a few comments and insufficient thinking :-).

Anyway, I had a look at your proposal, and I think that your changes to 
KTimezone/KTimezoneSource seem sensible: you have clearly thought about the 
customs use cases in more depth than I.

> >Although I have not had a chance to look at David's proposal, I would like
> > to say that I was also concerned by my version of KTimezoneDetails for
> > the reason David suggests. Perhaps his alternate design is more powerful
> > in this regard.
>
> One other detail to note is that my code also fixes the issue of
> recognising when /etc/localtime is a link, and also addresses the problem
> of re-reading zoneinfo files unnecessarily, by (1) checking the file size
> BEFORE it is opened, and (2) caching the checksums. As Nicolas previously
> pointed out, this is an important issue because of the potentially large
> numbers of files involved.

Well, I added the link following, but yes, I had not got to the other 
optimisations. You might want to consider adding the new comments in my 
version explaining the type of algorithm in each section of this logic: it 
might be useful to help others follow along.

One point I might make is the thinking behind my original splitting of 
KTimezoneDetails from KTimezone. I elected to keep this separate because I 
originally suspected that the vast majority of the clients of this code would 
not care about the capabilities supported by KTimezoneDetails, or worse you 
might be on a platform where this information does not appear to be present 
(i.e. Windows). You seem to have struck a nice midpoint with KTzfileTimezone: 
I like it.

In summary, I have not done a line by line review, but I like the progress you 
have made on this.

Thanks! Shaheed