[tz] FW: Barnaby

[Thom Hehl]

>You're using zone.tab as a canonical list of timezones, which it isn't.
>zone.tab is only useful for selecting timezones via countries; for a
>list of all timezones you can serve you should just look at the
tzfiles.

Did you read the readme? That is the next thing I'm going to fix.

>Your usage of time_t is not appropriate.  The type you want there is
>dictated by the tzfile format, not by libc.

The API needs to use time_t, so the usage is dictated by the use case,
not by the tzfile, which is why it's appropriate.

>You're doing a linear search (in transitionTimesIndex) that should
really
>be a binary search.

Yeah, that can be optimized. The problem with doing a binary search is
that you're not looking for an equal condition, but the smallest gap the
thing you're looking for fits into. I'm aware that a binary search would
improve performance some, but didn't devote time to it yet. Again, this
is a beta drop designed to get the job done so I can move on with other
priorities that pay money. :)


>Overall it looks like you're interpreting (the 32-bit part of) the
tzfile
>correctly, but I find the code too messy to be sure.

>You have some instance methods that should be class methods ("static").
>There is much else that is poor about your object model, but that's all
>off topic for this list.

Feel free to email me off list as you please for any improvements you
might suggest. As I indicated in the read-me there is much room for
improvement, but this meets my current needs. Open source is all about
refining code over iterations, right? Whatever changes you'd like to
suggest will be re-visited when I get some time to come back and work on
this more.

Thanks for the excellent feedback.

-----Original Message-----
From: Zefram [mailto:zefram at fysh.org] 
Sent: Tuesday, November 08, 2011 12:44 PM
To: Thom Hehl
Cc: tz at iana.org
Subject: Re: [tz] Barnaby

Thom Hehl wrote:
>http://code.google.com/p/barnaby

You're using zone.tab as a canonical list of timezones, which it isn't.
zone.tab is only useful for selecting timezones via countries; for a
list of all timezones you can serve you should just look at the tzfiles.

Your usage of time_t is not appropriate.  The type you want there is
dictated by the tzfile format, not by libc.

You're doing a linear search (in transitionTimesIndex) that should
really
be a binary search.

Overall it looks like you're interpreting (the 32-bit part of) the
tzfile
correctly, but I find the code too messy to be sure.

You have some instance methods that should be class methods ("static").
There is much else that is poor about your object model, but that's all
off topic for this list.

-zefram