Open source code quality

As part of my job I quite often work with OpenSource components in Delphi as they are invaluable time savers for getting my work done within a reasonable timeframe. There are many great projects such as the JEDI JVCL and JCL, Graphics32, DSpack and PicShow. These components work great most of the time however as with any code from time to time a problem will be discovered and some maintenance required. The problem comes whenever you dive into the code to take a look at what you can do to fix the problems you are almost always faced with poorly written, badly formatted and generally uncommented code. I appreciate that I didn’t have to write the components in the first place but at the same time so many of these components aren’t great examples of good coding practice yet hundreds of developers will rely on them. I take particularly issue with OpenSource projects because I’ve seen so few if any that I would say had a good coding style.

As a programmer I find the near universal lack of commenting very worrying as I personally think that commenting code is almost as important as writing it, for me comments represent writing what I think the code is or should be doing at the time I wrote it versus what I might later find out it isn’t doing. It helps to align my thinking with my execution. You learn about the importance of commenting every time you revisit a non-trivial piece of code after it’s faded from active memory.

The coding style is quite important for readability, take a look at this snippet of some code I’m looking at right now…

if assigned(FFilters) then
  if FFilters.Count > 0 then
    for i := 0 to FFilters.count - 1 do
      FilterList.Remove(IFilter(FFilters.Items[i]).GetFilter);

The above is perfectly acceptable compiling code but by leaving out the begins and the ends (Delphi’s curly brackets for all you C coders) in the nested code isn’t recommended for readability nor for the risk of introducing errors. Also it could be argued that the 2nd line in unnecessary as the 3rd line would prevent the 4th line from executing under that condition anyway. There are some other things I could pick at but you get the idea. I saw another example in the same file where the programmer had reused a TList (linked list class) variable and in some cases this ended up being created twice but freed only once leading to a memory leak, in fact the whole component was leaking a significant amount of resources which is why I ended up looking at the code in the first place. This may sound picky but when you start leaking interfaces and thread lists on 9x machines with low resources you at serious risk of taking a one way ticket to Crash City or BSODville.

I don’t want to berate the quality of OpenSource code nor the coding ability of the authors of the code, nor do I want to tar everyone with the same brush as I’m sure there are plenty of extremely well written projects out there. What I do want say is just because something works for you 99% of the time you should not expect that the code your interfacing with to be written to the same standards you work to or to be fault free more often than not it’s down to code never having operating outside the expected conditions. It’s also advisable that if you do write code that you release to the public you should uphold professional standards of coding practice because you never know when a prospective employer may want to use that code to evaluate your coding ability.