Lessons From Working With Timezones.

I have been reviewing, and then when the reviewing turned up a load of pre-existing bugs actually fixing the damn things myself, a lot of code to do with handling timestamps. Here are some of the lessons learned.

Use boost date_time.
It's one of the boost bits that comes in a library rather than just a header file but don't let that put you off, it makes easy some things that were hard, and makes possible some things that were impossible. It is a good thing.

The rest of these rules will focus on the more prevalent posix types and functions.

Never put anything other than posix time in a time_t.
By posix time I mean time since midnight 1st Jan 1970 UTC, aka epoch time, or unix time, or timestamp. You will have a hard time persuading anything to treat your time_t value as anything else so just don't do it.

Never put anything other than the local time in a struct tm.
Effectively this means never calling gmtime. There is only one reason to ever call gmtime, and that is if you need to get a UTC time string, in which case don't keep the value around for any longer than it takes to create the string. Doing so is asking for trouble.

Anybody writing code that relies on being run in a particular timezone shall be taken outside and shot.
This should be obvious and strikes me as lenient, personally I would beat them with sticks until dead. Shooting's too good for them.

Anybody writing tests that rely on being run in a particular timezone shall be taken outside and beaten with sticks until they are sorry.
It may look like an easy and harmless shortcut but No, just No!

Never go near the TZ environment variable.
Oh TZ environment variable, how I hate thee? Let me count the ways:
- You are not thread safe
- You can be changed invisibly by other parts of the codebase.
- You are hard to clean up correctly.
- You do not even bloody work properly.
Yes, it turns out that Windows is, as you'd expect, expertly engineered to work in every location you can imagine except if you change the TZ environment variable, after which it will get the timezones right (but of course), but likely get the dates of change from standard to daylight saving and back again wrong. it will use the American dates, even when it is not in America.

Alter your timezone and check it again.
I have come to the belief that writing time-handling code in a timezone that coincides with UTC for half the year is an actual disadvantage allowing bugs to go unspotted. Now, whenever I go near it, I tell my dev box it is in Road Town.


The weather's better too.

Different Ways Of Thinking

An engineer's mind is focussed on objective reality, it needs to be, it is the medium in which he or she works. When an engineer tells you they will do something it means they intend to do it, and believe they will be able to do it.

A manager's mind is focussed on subjective reality, it needs to be, it is the medium in which she or he works. When a manager tells you they intend to do something it means they agree it would be good if it were done.

When a salesman tells you they intend to do something, they are just making noises they think will make you happy.

How Code Reviews Lead To Crappy Code

I have finally figured out what it is about code reviews I do not like.

There is a lot to like about code reviews, they extend knowledge, they allow developers with little experience in certain areas to work in those areas with the confidence that any dumb mistakes they make will be caught, and most importantly, they genuinely catch bugs - often very nasty subtle bugs at that.

At other shops I have done reviews of the 'cast your eyes over this before I press commit would ya' form, and these were low impact and high value. Here we are having a real quality drive in an (I think successful) attempt to pull a legacy product back from the brink of becoming a bug-ridden mess, and are doing long, involved, line-by-line dissections of every commit.

Here is what happens, like any department anywhere, some people are better at the job than others. Always fond of a massive generalisation I shall divide this entire gamut of human ability into two sorts of people which I shall refer to as senior devs and peons. Quickly, I find, the department falls into the state where peons do most of the work and the senior devs review it. Senior devs would love to do the work themselves, and would do it faster and better, and would use things like Patterns, and Modern Language Features, and all the wealth of experience and knowledge that has made them senior; but they are too busy with their reviews.

Ahha, you might say, but peons code slower, and so produce less work to review, so it should even out, right?

Would this were so, but peon work is often poor, or more often sort of okay but with subtle errors and missing some corner cases. It gets sent back, with constructive criticism carefully worded not to offend, and then returns, with a few different errors, and cycles backwards and forwards, and becomes a mix of intents, of the parts where the reviewer has put her foot down and explicitly dictated a design (sometimes I think I should just copy and paste a diff into the review comments) and, and here's the rub, parts where she has not had the heart, or the patience, to criticise any further, and has allowed the peon to muddle through in their own way.

I didn't bother picking up on the stinky bits in the first review you see, because at that point it didn't work at all and a bit of a pong was the least of its problems, and I kind of assumed they'd go when the thing was rewritten to, you know, work. But they did not, and now, on the third cycle of the whole depressing business, it seems cruel to pick up on them, and with heavy heart I am forced to let them through because, stinky though they may be, I'm just happy that they work.

And so you end up with code which, while it contains no known issues, is just not very good; it is fragile, and has bad encapsulation, and the tests are close to useless, and frankly, it sucks. Meanwhile it has actually taken longer than it would have taken the senior developer to do the damn thing themselves, and they would have done it better, and they are wondering if the department wouldn't be more productive without the peon in the first place.