[PATCH] implementation of a timezone backend for Windows

[David Jarvie]

On Thursday 12 February 2009 23:14:38 Patrick Spendrin wrote:
> Till Adam schrieb:
> > Folks,
> >
> > as part of our work on Kontact on Windows in the enterprise4 branch
> > we've implemented a KTimeZone backend for Windows. You can find the
> > patch at:

I wondered when somebody might tackle this. I hope you had fun ;)

> > http://kdab.net/~till/e4-patches/timezone-support-windows.diff
> >
> > It's working ok, in e4, so we'd like to commit it to trunk, since it's
> > the only unmerged piece we have still. Didn't want to merge it before
> > 4.2 as we were not fully done with it yet. Patch applies fine to trunk.
> > David (Jarvie), please review, when you have time. Windows folk, maybe
> > you can also give it a read, in case we're doing something stupid,
> > Windows wise. Meanwhile I'll see about the unit tests on Windows etc.,
> > not sure they all pass yet.

I can only comment on how the code fits in with the existing code, since I'm 
not a Windows coder.

The first question is, does Windows have alternative methods of accessing time 
zone data? Unix allows time zone data to be accessed either by the libc 
library or by accessing the tzfile data. The former provides more limited data 
(and is used by KSystemTimeZone), while the latter provides all the system 
knows about time zones at the expense of extra overhead (and is used by 
KTzfileTimeZone). Your implementation would be suitable if Windows provides 
only one data access method, or if it provides more than one method but you're 
only using the 'easy' access method just now.

On the assumption that they provide the fullest information available on 
Windows, I wonder whether the KSystemTimeZoneWindows classes could perhaps be 
made publicly accessible in the same manner as the Unix-specific 
KTzfileTimeZone classes. Just an idea.

I do wonder whether implementation code in ktimezone_windows.h shouldn't be 
separated out into a .cpp file. It isn't particularly easy to read with the 
declarations and implementation mixed together. Either that, or use 'inline' 
declarations to allow all the implementation code to be moved to the end of 
the file.

The Windows code uses assert() calls to check passed parameters whereas the 
existing time zone code uses tests and returns a default value if the tests 
fail. Shouldn't they use a common approach?

I notice you use qDebug() and qWarning() instead of the KDE equivalents. Is 
there a particular reason for this? Aside from the more obvious effects, this 
prevents debug output using debug area 161 which is used by the other time 
zone classes.

kdebase/runtime/ktimezoned/ktimezoned.cpp needs to have Windows code added in 
the appropriate places as well. Or, if ktimezoned is by any chance unnecessary 
on Windows, it should be disabled on that system.

> Looks ok so far.
> maybe the test application could be removed from the comments and
> converted into a unittest instead.

I'd also move the test code rather than leaving it in comments (without 
explanation - seeing a commented out KTimeZone class declaration makes one 
wonder, what on earth is going on here? until one sees the commented out 
main() code later on). Or if really necessary, use conditional compilation 
rather than commenting it out.

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