It’s almost a truism that programmers would rather write new code
than modify existing code. The truth of that adage is doubled when
talking about modifying someone else’s existing code. Unfortunately,
that’s exactly what Tyler and I have been doing nonstop for the past
two weeks, and let me tell you, it has not always been pleasant.
After we got the source code the GPL component to compile in VS.NET,
the next few days were spent, more than anything else, simply getting a
feel for the code base. Different programmers have different conception
of what orthogonal design entails; the degree of separation, the
function of different components, the interfaces between classes, all
vary dramatically from coder to coder and company to company, and
invariably your conception is not quite the same as the fellow next to
you. Understanding the design of the existing base, then, becomes very
important to ensure that when you start making extensions, you do it
right.
The problem is that when you’re first getting started, the chance of
getting it truly right the first time—especially if you’re making
large-scale modifications—is very low. In Aardvark, for example, I
initially attached some new connection-specific handshaking code inside
a class that controlled the windows taskbar icon for our program. That
probably sounds a bit silly, but it actually made a reasonable amount
of sense. The taskbar class was where connections were created and
destroyed. Since that code was written for multiple connections, and I
was restructuring it to allow only one connection, adding in some
handshaking code into the new connection code there seemed as if it
made sense at the time. It also had the benefit of keeping all
Aardvark-centric modifications in just one class.
Needless to say, that was a bad call and caused problems down the
road when Tyler started adding encryption code and needed to hook it in
after the connection had been started and after Aardvark handshaking,
but before any other communications took place. So that had to be
refactored heavily. Thank heaven that Subversion lets you move files
while preserving history.
It’s doubly a problem when two coders are trying to come to grips
with the code base at the same time. While I was on one end of the
room, adding handshaking code and modifying the interface, Tyler was on
the other side of the room trying to make the connections use secure
sockets. That meant we were each trying to modify the same section of
code at once, and because we each had different concepts of what the
original programmers had in mind from a style perspective, we stepped
on each other’s toes a lot even with Subversion’s assistance to keep
things straight. Despite our best efforts, checking in code that worked
fine on one machine but that broke the other’s code base became a
frequent occurrence.
(Thankfully, Subversion has numerous features—such as repository-wide versioning that lets you revert the entire source tree
to the way it was at a specific point in time, powerful change tracking
tools to figure out quickly which lines of code broke the program, and
solid merge support—that made this far less painful than it could have
been. And, of course, Subversion integrates very well with FogBUGZ, letting us track easily which check-ins fixed which bugs. Without Subversion, it would have taken us much longer to get here.)
Even if we had instantly made all the right decisions, though,
stylistic variations would have hit us in another way. Large projects
tend to have multiple standard styles for different components. The TightVNC
project is a great example of this. vncviewer, which is the part of VNC
that lets you control other machines, codes directly against the
Winsock API in its ClientConnection class, whereas the sibling WinVNC
server has a VSocket class that abstracts all socket communication and
uses that class in vncServer and vncClient classes. WinVNC likewise
makes extensive use of Windows’ Unicode string manipulation functions,
whereas vncviewer codes against the ANSI functions. Classes in WinVNC
are normally called vncFooBar or VNCFooBar, whereas classes in
vncviewer are called just FooBar with no prefix. WinVNC makes extensive
use of a pattern for dialog boxes that relies on threads; vncviewer
lacks such a pattern.
This is not a problem in any real way; both code bases are solid,
fairly consistent internally, and most importantly, as anyone who’s
used TightVNC can tell you, very reliable. It’s just a bit confusing if
you’re working in a project that has two fairly different styles to
keep your coding appropriate, and the different high-level designs
increases the chance that you’ll make a bad design decision by applying
one project’s architecture to the other.
Would it be worth cleaning up? Almost certainly not. It would take a
lot of time to get everything refactored to have a consistent style,
and would come at no benefit whatsoever to the end-user. It does
perhaps emphasize, though, that large projects (including Aardvark,
thankfully) can benefit by having standard coding conventions to ease
moving around the code base.