Code or Death

Stack Overflow ("You CAN teach an old coder stupid tricks!")

2009-11-10T12:23:00.000-08:00

A quick aside: everyone makes stupid mistakes. I made one today that caused a stack overflow. In retrospect, it was insanely obvious, but when surrounded by 1000 lines of other code and comments, stupid mistakes can be quite good at hiding.

What happened was I wanted to add a transactional model to a class, so if there's a failure, the existing values can be put back in place through a Rollback call. Great idea, right? I also needed a convenient place to store all the fields, so I ended up with:

public class UrlProfile
{
    ... (Somewhere around 1000 lines of code, white space, and comments)
    private UrlProfile ProfileCache = new UrlProfile();
}

Okay then, create a new UrlProfile object. Voila! Stack overflow. (Of course my tests caught this....)

Twelve years in the field.... for shame! I just need more caffeine. Yeah, that's it! :)

Everyone makes stupid mistakes! I'm certainly no exception.

(The solution, BTW, was to put ProfileCache into a property whose accessor creates a new instance if the underlying field is null.)

Coding Style ("Other coders dis me / say my code is sissy! Why?")

2009-11-03T09:21:00.000-08:00

Before we get into the fun of actually writing code, I want to talk a bit about coding style. The simple truth is that for every developer out there, there's a different style of writing code. Some elements of this style are inherent in the developer and some are inherent in the language being used by the developer. So, everyone who brings this up will tell you something different and likely argue vehemently that his style is the best style one could employ. Who am I to work against such a time-honored tradition?

It isn't important for a single style be used by all developers, even within the same language. It is important, however, that the same style be used everywhere throughout a project, even if multiple developers are all contributing different components. Why? Because using a consistent style within a project promotes code readability. High readability eases development, integration, testing, and maintenance of the code. Coding style is therefore an important part of any discussion about writing robust software. So I'm going to talk a little about my style and why it's the best style one could employ!

Although the examples in this blog are written using C#, most of the concepts within are applicable to any language. To be ornery, though, I'm going to start out by talking about style issues largely tied to C-based languages.

Conditionals, boolean:
I always explicitly list the expected value in a boolean comparision. For died-in-the-wool C family developers, this syntax is verbose, reminiscent of BASIC, and, well, kinda wussy (hence the title of this entry). But there are methods to my madness, all intended to bolster readability.

Compare

if (ready == true)
{
  ...
}

And

if (ready == false)
{
    ...
}

if (ready)
{
  ...
}

And

if (!ready)
{
    ...
}

What's wrong with leaving out true and false? From the compiler's point of view (and that of many developers) absolutely nothing. But, commercial software development is not a leisurely stroll through the park. When you've been called in at 2 AM because some service has gone offline and goodness knows why, you want the deck stacked in favor of your being able to solve the problem as quickly as possible so you can go back to sleep. If when skimming over the code, you see a ! that isn't there or don't see one that is there, your understanding of the code is exactly the opposite of what it should be. And if that mistake means the computer has been told to do something stupid, you better believe it will happily do just that. So do yourself the favor and type in the extra six or so characters. You'll thank yourself sooner or later!

For C#, code written this way will always work as expected. But C, C++, JavaScript, etc., have an additional issue. C# will not allow an assignment statement within an if statement. This is not always the case for the other C-based languages. Leaving out an = can be as bad if not worse than leaving out a !. So what can be done? Set your compiler to flag assignments in conditionals as errors. If your compiler cannot be set to do that or if you're (shudder) using an interpreted language, reverse the order of the operands in the if statement. The statement if (ready = false) will compile (and always return true), but if (false = ready) will not. This allows the compiler to do more work so you don't have to! It's a minor hit to readability, but I argue that relying on the tiny and easily missed ! is worse.

Conditionals, while loops, etc., and braces:
I also always surround conditional blocks and the like in open and close curly braces, even if there's only one line of code within them. In all C-based languages, the following structure is legal:

if (ready == true)
  Console.WriteLine("Ready");

Now, imagine I also need to flip a flag if ready is true. I'm in a hurry so I forget that I need to add curly braces because I now have two statements in my conditional block. Worse, my evil editor does automatic indentation. So, I end up with:

if (ready == true)
  Console.WriteLine("Ready");
waiting = false;

Hey, we're doing some kind of resource sharing or thread synchronization thing, aren't we? Only now we just signaled that a thread is done using some resource when it isn't. Poof!-- if you're lucky. Otherwise the code may just behave... oddly... and require extensive debugging. The error may not be immediately obvious either, because our helpful little (evil) editor removed the indent that might have tipped us off.

The solution? Always use curly braces with conditional blocks and the like. Always. That way you'll never forget them. In fact, the moment I put the closing paren on the if statement, I add both the open and the closing curly brace before adding anything to the block. That also prevents me from including anything I didn't mean to include within the conditional block (the opposite of the problem above). Using curly braces every time produces more readable code, too, because it's more orthogonal to always use something than to only use it sometimes.

Conditionals, ternary operator:
This thing is just plain evil. Never use it. It's the only thing I always reject out of hand during code reviews. The problem with it is that it's just plain unreadable. Consider:

int numWaiting = (ready == true) ? 0 : 1;

This statement is so dense reading it is like looking for a full stop in a Charles Dickens novel. The code below is immeasurably more readable:

int numWaiting;
if (ready == true)
{
  numWaiting = 0;
}
else
{
  numWaiting = 1;
}

And this is (at worst only almost) the exact same thing from the compiler / runtime point of view, but a human can actually read it. And if there is a runtime difference here, we're talking nanoseconds. What are you doing using managed code if time of that scale matters to you?

All that being said, I absolutely love the C family for statement, despite its density, which one could argue is comparable to that of the ternary operator. The semi-colons separate the loop constructs cleanly and obviously, and I maintain it helps bolster readability to have all loop control elements on one line instead of (potentially) dozens of lines apart. It also helps guard against forgetting to add your loop counter increment statement, an oversight that usually results in an infinite loop. So remember, ternary operator = evil, for statement = good.

Comments:
I would say that mentioning code commenting standards in a room full of developers is like banging a cymbal near the summit of a mountain, except being buried by an avalanche of snow and ice is probably less painful. While there are many contentious style debates, the extent to which comments should be added to code can become one of the most heated. Some developers feel that lack of comments renders code useless, whereas others feel that code should be self-commenting, that it should be written in such a way that the code itself conveys its intentions to the reader. Sara J. Chipps, AKA Girl Developer, is one proponent of the latter approach. In her blog, she writes a scathing op/ed piece on the evils of comments called Documentation: A Sure Sign of Garbage Code. (And I thought I had strong opinions!)

The thing is, I absolutely agree with most of her sentiments. You really should write your code in such a way that someone could read it and understand what it does without needing to read extensive natural language documentation. You should make your code as readable as possible for all the reasons listed in the sections above.

And then you should comment it.

Why? Because when you work on projects with other people (and it is safe to assume that you will sooner or later) you never know what skill level they may possess. Sara says "[I]f your programmer needs documentation in order to be able to quickly pick up on what [your code] is doing one of two things are true: either the programmer sucks or, more likely, your code sucks." If you take care of business as described throughout this blog, you ought to be able to rule out the possibility that your code sucks. The problem is that oftentimes the other programmer does suck. Or, to put it more politely, the other developer may not have the mad skills you posses, but (s)he may still be the one who has to make modifications to your code down the road. Do you really want to get called in every time someone with a lesser skill set than you have needs to work with your old code? Especially at 2 AM? If not, then it's in your best interests to arm the other developer with all the information you can to stack the deck in favor of you never receiving that call.

Another reason to add comments lies in the ability to easily convey interfaces to other developers who intend to leverage your code. The beauty of XML comments within C# is that anyone can use these to build an MSDN-like knowledge repository for how to leverage their libraries and services. If you comment your code appropriately, this becomes something you can provide people for free-- no extra effort on your part to write separate documentation. Otherwise, you either have to take the plunge and write separate documentation or get interrupted every time someone else has a question regarding your code. And, as you should know, there's a lot of overhead in servicing interrupts.

So, then, in my humble opinion, what kind of commenting is needed? It varies from project to project, but in general, I do the following:

File header:
A file header tells people when they open your file what they can find in that file. If they read the header (and, yes, there's always the possibility no one will bother to read the comments) they should know whether the file might contain what they are looking for. This can be extremely helpful when your project is of sufficient scope that it contains hundreds of files. An example file header:

//************************************************************************************************************************************
// System: XML Communication Sample
// Subsystem: Web Services
//
// File: XmlImporter.cs
// Purpose: Contains the XmlImporter class which is used to import XML blobs into the store.
// 
// Copyright (C) 2009 Gene DeClark. All rights reserved.
//************************************************************************************************************************************

The copyright notice is important if you want to protect your work. Note that if you are writing this code for pay, this notice should typically contain your employer's name instead of your own. If you happen to be copyrighting your code via a certain kind of license, such as the GNU Public License, you should note that here. If you (and your employer) don't care if others directly leverage your source code, though, you don't need to include a copyright notice.

Note that the above is not XML and is not included when XML comments are built into documentation files by Visual Studio. That's because from an interface point of view, source files are irrelevant. But someone actually looking at your project will benefit from you including these file headers.

Finally, the "borders" can certainly be omitted if you prefer.

Class headers:
A class header tells people the role your class plays within the overarching product. Because class headers are included within XML comments, they're automatically built into documentation files by Visual Studio. Further, when the Visual Studio IDE is used, context-sensitive tooltips will show these comments to code readers and writers when appropriate. So your documentation effort pays off even if no one sets out to read the docs. Very cool! An example class header:

/// <summary>
/// Provides functionality to import XML blobs into the store, including schema verification via XSD, creation of new blob
/// entries, and existing blob updates.
/// </summary>
public class XmlImporter

Method, property, etc. headers:
XML comments can also be used to create headers for methods, properties, and the like. Of all headers, these are the most important, because they tell other people how to make calls into your code. Leaving these comments out is a good way to ensure questions get sent to your inbox (or your cell phone) at the worst imaginable times. The XML comment schema for these elements is fairly rich, so you should spend some time researching the possibilities. An example method header:

/// <summary>
/// Deserializes the specified data into a new XmlImporter.
/// </summary>
/// <param name="serializedData">Serialized data to deserialize into a new XmlImporter.</param>
/// <returns>XmlImporter populated with deserialized data if successful. Else returns null.</returns>
/// <exception cref="NullReferenceException">
/// Parameter serializedData is null.
/// </exception>
/// <exception cref="ArgumentNullException">
/// The OuterXml property of parameter serializedData is null.
/// </exception>
/// <exception cref="InvalidOperationException">
/// Deserialization failed. The InnerException property contains further information on the failure.
/// </exception>
public static XmlImporter Deserialize(XmlElement serializedData)

Note the list of Exceptions.... We'll talk about those and the role of method preconditions (which can be included in the <remarks> element) in more detail in a later entry.

Inline comments:
So now that we know what each file does, what each class does, and what each method, property, etc., does, what else do we need? Well, we need to understand what the code inside the method does. If you follow all the other advice in this entry, you may be able to add comments inline within your code very sparingly and be just fine. I still elect to add comments rather liberally so that people unfamiliar with the code can jump in unaided (assuming they understand the language I write the comments in, which also, granted, may not be true). So, when do I add an inline comment? Generally, before any loop or branch and before a section of code that performs a portion of a larger job. For example, assuming serialization code was short enough, I might serialize an entire object within a single method. Since each property is a discrete portion of a larger job, I might include a comment above each property's corresponding lines of code. Or I might not. Whether I do or not depends upon whether I think that anyone I work with or may work with in the future might be confused if I don't add the comment. So in general, I add the comment.

Declaration order:
Once upon a time, back when C was king, elements had to be declared in a certain order. Variables had to come before functions, functions that were called by other functions had to be declared first, and so on. The problem was that all the minutiae always came first within a file. The really important code was always further down. This approach also could cause problems in other scenarios, such as mutual recursion. Along came C++ and the concept of classes. Within C++, the class declaration and implementation were in separate files. So, the order of declaration still mattered, but at least the minutiae was out of the way. When C# came out, however, separate declaration and implementation files went the way of the dinosaur, and all the minutiae once again ended up at the top of the class, above the really important code.

The thing is that order of declaration doesn't matter in C#. So long as a reference can be resolved at build time, declarations can come in any order. So it makes no sense to include all the minutiae at the top of a class. Instead, I always organize my code so that the most important elements appear at the top. This usually means that construction and destruction come first, followed by public properties, then public methods, followed by interface implementations, then protected and / or private methods, followed by protected and / or private properties and fields, and finally constants. Everything is easy to find thanks to Intellisense, anyway, but with this arrangement, the code someone most wants to see when they open the file is usually right on top.

As an aside, it is possible as of .NET Framework 2.0 to split elements of classes between files through the use of partial classes. If you see people doing this, they're probably died-in-the-wool C++ programmers! You can indeed separate some elements of a class into separate files, and there are times when this may be a good choice. But, generally, placing class minutiae into a #region declaration will sweep it under the rug sufficiently that the overhead of maintaining a separate file isn't really justifiable.

Method entry and exit:
A method, in general, can only be entered at its beginning. It can have any number of exit points, however. This is the potential source of much gnashing of teeth in the world of unmanaged code, but don't be fooled into thinking managed code solves this problem. That way leads to suffering even for C# programmers. Consider:

/// <summary>
/// Used to specify the result of an attempted cache update.
/// </summary>
public enum CacheUpdateResultCode
{
    /// <summary>
    /// Indicates the cache update operation succeeded.
    /// </summary>
    Success,

    /// <summary>
    /// Indicates the cache update operation failed because the specified file could not be found.
    /// </summary>
    Error_FileNotFound,

    /// <summary>
    /// Indicates the cache update operation failed because the specified cache key could not be found.
    /// </summary>
    Error_CacheKeyNotFound
}

/// <summary>
/// Cached file contents.
/// </summary>
public static Dictionary<string, string> FileCache = new Dictionary<string, string>();

/// <summary>
/// Updates the cached copy of the specified text file.
/// </summary>
/// <param name="filePath">Path and name of file whose contents to update in the cache.</param>
/// <param name="key">Key of file to update in the cache.</param>
/// <returns>Result code of the cache update operation.</returns>
public static CacheUpdateResultCode UpdateFileCache(string filePath,
                                                    string key)
{
    // Make sure the file exists.
    if (File.Exists(filePath) == true)
    {
        // Open the file and read in its contents.
        TextReader textReader = new StreamReader(filePath);
        string contents = textReader.ReadToEnd();

        // Update cached version of file.
        if (FileCache.Keys.Contains(key) == true)
        {
            FileCache[key] = contents;
        }
        else
        {
            return CacheUpdateResultCode.Error_CacheKeyNotFound;
        }

        // Free resources.
        textReader.Close();
        textReader.Dispose();
        textReader = null;

        return CacheUpdateResultCode.Success;
    }
    else
    {
        // The file did not exist, so return false.
        return CacheUpdateResultCode.Error_FileNotFound;
    }
}

Look how responsible we're being! We're checking if the file exists (which also indirectly checks to see if the filePath parameter argument is null or empty), we're cleaning up the resource, heck we're even setting textReader to null when we're done. And we commented the HECK out of the thing. So what's wrong?

Several things, really, many of which I will talk about in the entry on exception handling. The one I want to focus on right now, though, is that this code includes three return statements. On the surface, this doesn't appear to cause any harm. But, if the file is read successfully but the key is not already in the Dictionary, the method exits without cleaning up the file resource. If this code path is executed frequently, congratulations, you've got a bad memory leak. This happens because files are unmanaged resources accessed through managed classes. If you don't call Dispose, the unmanaged resources aren't freed. One way to help ensure Dispose is always called is to make sure a method only ever has one exit point. This, combined with exception handling and the using pattern, both of which we'll talk about in a later entry, will ensure Dispose is always called (barring power outages and the like!).

Cohesion:
Files, classes, methods, in essence all elements of a program should be highly cohesive. They should perform a single task, model a single entity, and so on. And they should be as short as they can be. Specifically:

A file should, in general, contain only one class, interface, etc..

You will see me break this rule on occasion. (Naughty, naughty!)

A class should represent a single entity, its state, and the actions that entity can take.
Methods should perform one small function and nothing more.

As a corollary, methods should be limited in length. If you're approaching 100 lines of code (including comments and whitespace!) or if you have so many loops and conditionals your lines are starting halfway across the screen, you should strongly consider refactoring your method into smaller functional units. Don't be afraid of private helper methods. And don't even talk to me about function call overhead-- this is managed code for ELIZA's sake!

GOTO:
Remember how I said earlier that the ternary operator is the only thing I ever reject out of hand in a code review? That's because no one's ever had the audacity to hand me code with a goto in it. (Please understand that I'm not including within this discussion languages that don't include control structures other than goto-like statements such as assembly and line number BASIC. We should all feel dirty for ever having used line number BASIC, anyway!)

In my personal opinion, if Dante's Inferno had included a tenth circle of hell, its sole occupant would be the goto statement.

Using gotos is the fastest, surest way of producing unreadable, unmaintainable, incomprehensible code. But don't take my word for it. Read, for example, Edsger Dijkstra's letter A case against the GO TO statement written over 40 years ago! Beyond this, though, even further back in history it was proven that languages that can model structured programming (which all C-based languages can do, even the venerable C) have absolutely no need of gotos.

So, if this debate was settled over 40 years ago, why am I bringing it up? Because it wasn't settled. Some important people in the industry still hold that use of gotos, in at least some cases, is better than use of structured programming. One such proponent is Linus Torvalds, who is best known for having started a little project known as Linux. If you follow the link above and read the argument, he is at least partly basing the defense of gotos on the fact that some languages such as Pascal don't have a break statement. It's true that when I was coding in Pascal some 20 years ago, the lack of a break statement did create an occasional lack of elegance. But this isn't 20 years ago. And we're not using Pascal. And all C-based languages do have a break statement. So I don't think it's a very strong argument. Still, more people know Linus's name than mine, so I really have to present that side of the argument as well. I would encourage you to do your own research and decide for yourself. Just don't ask me to review your code if you use gotos. My constitution is too delicate for such rot.

Conclusion:
Wow, this is a long entry! You should know that I've spent a long, long time developing a style. Your style will undoubtedly differ from mine. So long as you have defensible reasons for why you use a particular style, have at ye! And be prepared to evolve your style over time and to be surprised by some style standards you learn about. Just the other day, for example, I learned that it's considered preferable to have using statements inside the namespace block. I'd never seen this before and hadn't considered it. But, I can't think of any reason not to do so, and doing so just plain feels like an improvement. So guess what I'm doing with my using statements now? My carefully developed style got slapped upside the head and I adapted. You'll need to do so, too, so don't be surprised when it happens!

Robust coding ("Welcome! You've got bugs!")

2009-10-02T14:27:00.000-07:00

"Raise your hand if you love coding!"

     Lots of hands, waving enthusiastically.

"Raise your hand if you love debugging!"

     Fewer hands, a little less glee.

"Raise your hand if you love testing!"

     A few, limp-wristed hands.

"Raise your hand if you love regression testing!"

     Crickets chirping, quiet coughs. Some guy in the back gnawing his hand off at the wrist.

I by no means stand alone in my passion for the production of high quality software. I also by no means stand alone in finding large portions of software testing, well, boring and tedious. For six long years, I worked full time as a software tester. I spent most of that time wishing I were writing the code instead of testing it. In my experiences within the industry, until recently there were testers and there were devs, and seldom was one successful in moving from one of these camps to the other. The industry as a whole, however, is changing, and even large companies are moving away from placing rigid walls between development and testing efforts. It's finally becoming accepted that to write good code, you must understand how to test it and execute on that need. It's also finally becoming accepted that to thoroughly test code, you must understand the code you're testing. This is certainly not what I encountered when I entered the industry twelve years ago.

"But I thought this blog was going to be about coding!" someone mutters. "What's all this rot about testing?" It's a fair question. Software testing is a complex and deep discipline, and huge swaths of it don't involve writing a single line of code. I have to stand by my earlier assertion that you can't write quality code without understanding testing, though. What is a blogger to do?

Define terms, of course! Within this blog, we're going to concentrate primarily on white box (which sometimes goes by my perferred term glass box) testing and relegate discussions of black box testing to the background. Consequently, we're going to look at methods for driving the code we write with yet more code. To do this, we must be able to see, understand, and access our production code with our test code, hence the glassiness of our box of interest. The contrasting black box approach is driven around the idea of exercising production code with no understanding of its implementation whatsoever. Someone new to development might wonder which is the superior approach. The answer is that neither is better or worse, and both are essential. Each complements the other, providing knowledge about the code we're writing the other can't provide. In this blog, we're going to talk mostly about glass box testing, but I want to be clear that this focus is by no means meant to diminish the importance of the alternate side of the testing coin.

In the next few posts, we're going to write a small application that will import and export XML from a web service, using C# and the .NET Framework, version 3.5. So, we'll be using managed code and concentrating in part on the problems solved and the problems created through its use. More importantly, we'll write robust code, and we'll have a much improved understanding of the code's quality because of the test code we'll write as we go.

One day, producing quality code may be as simple as loading a web page and waiting to hear "Welcome! You've got bugs!" But it is not this day. This day we FIGHT!

"You: code or death?"

2009-09-01T12:07:00.000-07:00

I was doomed to geekdom at a very young age. One of my earliest memories is watching Star Trek with my dad, every Sunday at 4:30 on Channel 12. More so even than the sometimes positively educational costumes and weird aliens, the library computer absolutely fascinated me. Here was a marvel: a machine that would answer your questions, drawing on a repository of facts beyond those accessible to any one person anywhere. Being, as I was, a toddler with "why" and "how" on my mind more than anything else, this computer thing seemed a godsend. But even at that young an age, I knew what the word "fiction" meant, and I'd certainly never seen anything like that anywhere in the real world.

Fast forward to 1985, when I was in the fifth grade. After yet another lesson in square roots, decimals, rounding, or some other mundane thing like that, my teacher wheeled a cart into the classroom.  On this cart was a real-life, honest to God, working, living, breathing (okay I made those last two up), freakin' computer! I couldn't even hear the choir of angels singing in the background over the blood rushing through my veins at the sight of the thing. A Commodore PET. With integrated cassette storage. And I got to ask it questions. I will never forget that moment.

Okay, sure, this wasn't the Enterprise, and the Commodore PET sure wasn't a library computer. Even by today's standards it was really just a toy. In truth, its software was so limited you couldn't ask it for much information, really. It taught me the state capitals and the countries in Latin America. I soon learned, though, that not only could the computer answer my questions, but I could tell it what to do and it would do it! That machine sparked a lifelong passion within me. I now tell machines what to do for a living, and I learn more from them every day than I ever really thought would be possible.

This blog isn't about my story, really. I'll write more about that over time, sure, but what I really want to write about is code. Being able to tell computers what to do is, well, neat! But the problem is that they'll do exactly what you tell them to do, even spectacularly stupid things. This complete obedience, while damn cool, comes at a price. The role computers play in people's lives is growing in magnitude at a startling rate. When computers do exactly what we tell them and we tell them to do something stupid, it's now disrupting people's lives. Depending on what the software does, it may even cost people their lives. It is critical for the software industry to begin to ask (and answer) fundamental questions about how we can be sure what we're asking these machines to do is what we actually intend them to do. Questions like "what separates 'good' code from 'bad' code?" are a start.  "What should we be looking at to figure out what our code is actually doing?" may be a better question.

What this all comes down to is a combination of technologies and best practices, both of which are changing every day and will continue to change at a profound rate. Remember that humans have only been developing computer software for about 65 years. How good were we, really, as a species at building bridges 65 years after the first ever bridge was built?  Would you drive your car across that bridge? I sure as heck wouldn't. While the methods of sharing information and therefore pushing the state of the technology forward are unquestionably more advanced today than they were back when bridges were still a pretty neat idea, this still means that no one really knows how to write software you could drive a car over. I sure as heck don't. But I endeavor to improve the quality of the code I write relative to the quality of code I've written in the past every time I sit at the keyboard. I've learned some useful stuff over the years that I can share. Many people will disagree with much of what I write in this blog. I wouldn't have it any other way. Only by questioning what we're doing can we get better at it. I hope this blog is helpful in that process.

If you've read this far, you deserve the answer to a question: What the heck is up with the title of this blog? Eddie Izzard, a British, cross-dressing standup comedian, once did a routine in which inquisitors went around asking their victims if they wanted cake... or death. Surprisingly, everyone wanted cake, and they ran out. So their remaining victims were given the option "or death." Don't worry-- there's still plenty of code to go around. :)