Code Quality
Over the time the code that we're writing undergoes (or is supposed
to undergo) a tremendous change in quality. Just for fun I recently
rewrote my ancient GIF image encoder/decoder the way it should've
been written. I took into account most of the things that were wrong
with the initial implementation and the new version turned out to be:
- more flexible and genereal (can work with tightly packed raw data buffers)
- more portable (compiles with a variety of 16/32-bit x86 C compilers)
- more reliable and secure (most of (if not all) checks are in place)
- thread-safe and instantiability is trivial (that is, no globals modified)
- faster (proper use of the hash table in the encoder)
- about twice as big at the source level
All of the changes were influenced by the experience gained during
several (5+) years of professional development of software in C.
Compare side by side:
Old code (from around 1999) | New code (2007) |
|
|
A few comments and observations:
- There's little technical difference between the two implementations.
Most notably, the new interface doesn't work with files. Instead, the
memory is used. This is a more general-purpose implementation that does
not require the GIF source or destination to be a file. Actually, it
fits nicely with memory-mapped GIF files.
- Space-wise the two implementations are different. The old
implementation used less memory for the algorithm itself at the
expense of the speed and it always used a byte per pixel for the raw
unpacked image. The new implementation uses more memory for the
algorithm and enjoys a better performance because of that, but it can
also work with packed images which may need less memory (here by packed
I mean that if a pixel has fewer bits than the 8 bits of a byte,
the rest of the byte bits aren't simply wasted - they contain bits
from other pixels).
- While I could put in comments next to the main API functions to
describe the parameters and some implementation details, I decided not
to do it because the intention wasn't to show the mastery in commenting
or English. It was to show how the actual code can be improved.
- For the same reason as above, the new implementation does not
include the support for interlaced GIFs. It wasn't the point to implement
an identical thing and the missing feature is pretty trivial.
- All of the improvements came at the price of the increased size and,
perhaps, slight readability degradation (due to the size and extra sanity
checks of the input data (parameters, GIF structures and even compressed
data)). But the code was never meant to replace the document describing
the GIF format and decoding/encoding operations anyways. So, it shouldn't
be a surprise that quality software is bigger and sometimes slower than
the low quality software.
- There're many ways to improve the code and there's no limit for
perfection - it can be pursued indefinitely. There could be further API
improvements, functional improvements (e.g. support for animated GIFs)
and size or speed optimizations desired in the application.
- Oh, btw, I'm not claiming the presented code bug free. I only tested
it on several dozens of images and I didn't try to break it by feeding
it with crafted, intentionally malformed data. I tried to do my best
in determining all the necessary validations, though.
So, as you get smarter and more serious about the code you write, you
should be seeing a similar transformation. If you don't see it happen,
then either you're not getting smarter or you aren't any serious about
the code quality - you just plain don't care. The latter is sometimes
OK. For exameple, the code is temporary; it will be used once and then
thrown away. Or there're some guarantees about the inputs and you
don't need to be validating everything scrupulously. But we all know
it probably too well: our judgement isn't perfect and assumptions
aren't always right and just like there's nothing everlasing, some
things that we intend to be temporary solutions (aka quick and dirty
hacks, workarounds, kludges and so on) end up being used for a very
long time, which may make us unhappy in the end of the day.
Alexei A. Frounze
Feb 2008