Writing Clear Code

This topic is a bit of a divergence from Custom Actions, but useful for some general coding skills. I'm going to highlight the C# language in this post, although you don't need to know C# to benefit from this discussion - I'll cover the essentials in the post. The topic is how to write clear code that makes it easier to defer the understand the intention of the code rather than the behavior of the code. The subject came up about a few months ago in a discussion with a coworker who initially didn't quite agree with me. I'll lay out the argument here as I did with him.

Take for instance the following code snippet:

string a = "hello";
string b = "hello";
System.Console.WriteLine(a == b);
System.Console.WriteLine((object)a == (object)b);

Lets explore the last two lines. The a == b equivalence operator is working on strings, and by C# rule, two strings are equivalent if they are both null, or if both values are non-null references to string instances that have identical lengths and identical characters in each character position. Translation - two nulls are identical, and any two strings that match, case sensitive, will result in a true expression. In the latter comparison, when we cast both of the strings to objects, we are not comparing the values of the strings, but the objects themselves.

What do you think it will print? The answer is True and True. Why? String literals that are identical within the same assembly refer to the same instance. Translated, 'a' and 'b' are variables that refer to the same underlying object.

Now lets replace the declaration and assignment for string b with:

string b = "he"+"llo";

Now what do you think it will print? The answer is again True and True. Why? Because the Lexical analysis (lexer) in the compiler stripped out the needless additive operator in the literal. The same would have happened if we changed the string to be "hell\u006f" - since '\u006f' is the Unicode escape sequence of the lower-case letter 'o', and this expansion takes place (most likely) prior to the lexer during the transformation phase in the compilation process.

Now lets replace the same line as above with the following two lines:

string b = "he";
b += "llo";

Now what do you think it will print? The answer is now True and False. Why? The strings are equivalent, but since the preprocessing of the source file before the compilation did not identify the strings as being the same literal, they are in different objects. In fact, because a string is immutable (once created cannot be changed), there was the construction of the string "b" during declaration and initial assignment that was later garbage collected when a new instance of "b" was created during the string concatenation.

Now that the necessary background information is understood, can you tell me the intention of the following code snippet?

string a,b;
//code here that assigns and manipulates a and b
...
if (a==b)
    compareOK();
else
    compareBad();

I'm hoping you are going to tell me that you have no clue as to the intent of the programmer. The intent could have been to compare the two strings in a case sensitive manner, ignoring cultural rules (which is what actually is happening in the above snippet). It also could have been to compare object equivalence, just that the programmer forgot to cast the strings to object first. The intention could also have been a case-insensitive comparison. Another possible intent was to compare strings using cultural rules. The intent of the programmer simply is not clear!

Rewriting the comparison line in the above snippet correctly, assuming the actual behavior of the code was the intent should look like this:

if ( String.Compare(a, b, false,
System.Globalization.CultureInfo.InvariantCulture ) == 0 )

This tells me that the intent of the programmer was to compare the two strings in a case sensitive manner, ignoring cultural rules . There is no other possible intent given the above line of code. This is not saying that the code is correct - just that the intent of the code is clear to those lucky enough to read it, and that the developer thought it through.

If I am skimming through some code and see a non-String.Compare() string comparison, I will sprinkle a //BUGBUG: comment above it. Why? The simple string comparison syntax has been a rather common cause of bugs in C# code - much like the C/C++ switch statement. When investigating an issue in a piece of code, I will first look for these BUGBUG's and the TODO's as a starting point - often with better-than-random chance results.

Code Quality and the "Total Software Product Quality Triangle"

Today I came across "The Code Quality Myth" by Frank Sommers. Several quotes in the article really hit home. I could have written some of this essay. For instance, "I have almost never met a developer working on real-life, production software in a business environment who was completely satisfied with the code he was working on."

The code I am "proudest" of is typically short, "one-offs". This is typically stand-alone software or routines that serves one tiny purpose and is not tightly integrated into a larger product. Usually this software is not written under deadline, and is almost always a version 1.0 or a complete rewrite with few aspects of backwards compatibility. I am usually very proud of software when I have time to "do it right," and the quality is generally the best possible that I can produce.

Frank pretty much echoes my above sentiment with "I almost discern an inverse relationship between how satisfactory a piece of code is to the developers working on that code, and the amount of money that code makes for those developers' employers." Commenter Robert to that essay asked for metrics on that statement - I think he misses the salient points I am about to cover here.

Frank later talks about how the source code is not what we ship, and that only the binary matters - since this is typically all that the customer sees. This is where our opinions start diverging, and Frank misses one point that is staring him in the face: Why is this the case?

After 1.0 is released, or when that pristine little one-off class is integrated into a larger program, things start to go sour. Now we have to start working on new stuff and maintain backwards compatibility with the old stuff. It may be quite clear that the current API or XML Schema is not perfect for the new feature. Rather than refactoring or rewriting, we (as developers under a deadline) merely attempt to shoehorn the new feature into the old codebase. This is not perfect, pretty, or something to be proud of - but we think that we will be able to "do it right" for the next iteration, whose time ultimately rarely ever comes.

Besides the "shoehorning" there are other issues. Perhaps you want to move from the Old Hungarian into the New Hungarian. Perhaps there is a new "coding style" that you are supposed to use from now on. Different programmers working on the same class introduces some additional considerations. Maybe all of your string manipulation functions from today forward are to use the new "strsafe.h" functions. Whatever the reason, this results in fragmentation of your codebase. Some of it is the "new" way, some of it is the "old" way, and you get that massive maintenance programmer headache when attempting to context switch between two or more different coding philosophies in the same routine! Are you proud of this mess - heck no! Is this improving overall code quality and maintainability? Likely not!

The last thing that spices the code up a bit are platform differences and/or new technologies. Maybe you are porting a single class at a time to UNICODE and have ugly #ifdef's all over the place. Perhaps you want to take advantage of http.sys in XPSP2 and Server 2003 while still working under OS's that don't support it. Plus, that hack-around for people with IE4 installed is still lurking in the bowels of the core program. While this may assist in code "robustness" it certainly does not contribute to overall quality or maintainability.

In short, ALL source code gets uglier with time. For many reasons it has to. A solid developer knows that he or she can do much better in a rewrite with hindsight as our guide. An experienced (pre-agile) developer will architect classes with some degree of foresight (and bloat) to help mitigate this. The experienced agile developer (working with an organization that uses the agile philosophy) will write the bare minimum such that the affected classes need to be refactored for any major change to mitigate hacking around.

Planning for dealing with these future changes is what can help lessen the ugliness of the underlying code. In my opinion, the agile refactoring philosophy is one of them. Too many development organizations fear refactoring as "introducing risk" from 1.0 to 1.0.1 instead of fearing future effects of hack-arounds. This fear is unfounded (since it is easily mitigated with automated unit testing), and the tradeoff in having quality code vs. unmaintainable goo to propagate forward into the next major version is a no-brainer. Consider that the 1.0.1 fixes will be integrated back into the mainline. Is the fix you are making the correct one for the future? Does the fixed code look the same as it would have been written in 1.0 had it worked?

This is where Frank and I start converging again. "...total product quality, as Edwards Deming noted, is not the result of merely improving the quality of a single activity, but is rather the outcome of a set of processes focusing on the quality of the total output... Developer testing, agile development methods, quality assurance, continuous integration - these are all processes that facilitate a high quality of total output." We begin to diverge from there almost immediately.

To use an analogy that Steve McConnell did not, overall software quality is similar to the "Fire Triangle." To have fire, there needs to be Oxygen, Heat, and Fuel. Note that you can have all three without actually having fire - as fire is the result of the chemical exothermic reaction that occurs when all three of the other elements are present under the right conditions. Once fire occurs, take any one of these elements away, and there is no more fire.

In software development, I propose the "Total Software Product Quality" triangle. The three components are "Solid Code", "Complete Unit Testing," and "Continuous Integration Testing." In the presence of a "solid management process" that combines the three elements under the right conditions, we can have "Total Software Product Quality." Take any one of these elements away, and there is no more total quality - only fires that will need to be put out later at a tremendous cost.

What is Excellence in Software Development?

I recently attended an awards banquet, and after thinking about the events that transpired, I had to sit down and write this entry. One of the VP’s who was honoring the award recipients had a great concept buried in his speech that I am now paraphrasing – that we need to recognize everyday excellence, not just the people who are acting in “Firefighter mode.”

What are “Firefighters?” In any company this type of stuff happens more than we would like to admit. The situation may be a big demo. Six hours before the demo, your equipment arrives. What remains of the shipping crates looks like it was used as landing gear for the transport plane. After selling your soul to the devil, you manage to replace the damaged equipment with parts from an automatic toilet flush valve and pull off a successful demo. Is this excellence? Sure. Is this everyday excellence? Hopefully not. Stories of Firefighting make for great press, but are usually not indicators of overall excellence.

Shortly after the firefighting speech, a video was shown of an individual praising a particular developer. There were some great one line examples of this individual’s abilities – and the person was truly deserving of the award based on these alone. What got most of the video time was related to this developer’s rewrite of what was accepted as a truly terrible piece of code. We have a typical story here: Developer worked more hours in a week than most work in a month to get the rewrite completed. Is this a sign of Excellence, or is it an example of Firefighting?

My answer to this question is a resounding NO for both. If you have to work excessive hours for days/weeks/months on a scheduled project, it is the exact opposite of excellence. In industry terms, this is known as a Death March. Someone, somewhere screwed up bad. Perhaps it was the design. Perhaps it was the implementation. Maybe the fault was in the management expectations. Regardless of cause, this is a huge, flashing neon sign that something is horribly wrong. Perhaps we all need to read (or re-read) Debugging the Development Process: Practical Strategies for Staying Focused, Hitting Ship Dates, and Building Solid Teams by Steve Maguire. In my personal experience, EVERY project that required long hours by the developer ultimately failed for a multitude of reasons and with a multitude of negative outcomes:

• QA is often scrunched to get the project out the door.
  
• Because of the above, there are serious undiscovered bugs that get shipped.
  
• The resulting code is difficult to maintain.
  

Although I have no personal experience with this project and its outcome, anecdotal reports from people I spoke with confirmed the above bullet points. There are plenty of references to this topic, most of which are buried in research and justifications to development methodologies known as “agile.” While describing agile methodologies and concepts is outside the scope of this entry, one interesting paper worth a read that supports my assertions is Estimation as hypothesis, by Alan Shalloway. I highly doubt that anyone would want to encourage this type of coding behavior by using it as a justification for an award! Although up until now I have covered what is NOT excellence in Software Development, but I have not covered what IS excellence of the everyday kind.

Boneman’s Principles of Software Excellence:

• Know your customer - This is an extremely vague topic, but if you are developing software that provides simplistic configuration of widgets for Biscuit Welders, and most Biscuit Welders have a preschool education, your interface and documentation must reflect that understanding. This may involve following your customer around during one of their typical workdays to understand their job and their workflow. Any employer that does not allow, encourage, or suggest this during a new employee orientation and thereafter as required is not worth working for.


• Know and follow best practices - These days, security is a primary concern. Does your application comply with industry best practices? Do you know that such things exist? When was the last security related continuing education session held at your organization. If this answer is greater than one month ago, the developer with the most excellence will be scheduling one – hopefully this person is you.


• Compliance with current industry coding standards.


• Evangelism - Are you talking up industry standards and best practices with your teammates? Are you using code reviews as opportunities for education and not as a forum to criticize? Do you even have code reviews?


• Non-use of Programmer Quotes - You correct (harshly but constructively) any developer who utters one of the quotes found on this page because your code understand that all user input, data, or machines are evil until proven otherwise.


• Take the blame - If you get a call from a support person, it is almost always your fault, and you admit it. Most often this is due to the lack of error checking or useful error messages in the code you wrote. Instead of merely answering the question or fixing the problem, immediately adjust the code in question to provide useful feedback to the user. While you are at it, you write a Unit Test that exhibits this behavior.


• When you are stuck, you admit you are stuck - If you are behind schedule, you immediately admit it and ask for help. Recognizing that you or your project is in trouble is the first step, asking for help is what separates the men from the boys. Remember that you are part of a team. Even if you are the sole software developer in the organization, you need the support and understanding of your team to get through any coding crisis. This does not make you any less of a programmer, in fact, the ability to admit you need help will earn you my vote for a place on my team, anytime, and anywhere.


• Check with the helpdesk, implementation, and training personnel for issues related to your code - Every successful pro athlete reviews his or her performance on video after the big game. Why? So the athlete understands what they did right, and what they did wrong. In the world of software, all of the above listed people are the first line of customer contact. If your code nailed the extremely complicated but infrequent widget refactoring process you stayed up late thinking about, but bombs when previewing the daily report unless the user is wearing a green hat, you need to know this, correct this, and put into place a process to prevent these simple but frequent (not to mention annoying) bugs from happening in the code you are writing today.

The above is not even close to a comprehensive list, but rather what was on the top of my head as I created it. Sadly, I can’t think of a single person that within the last six months has actually lived up to this list, including myself. It looks like some things are going to change around here – and for the better. Please feel free to add to this list in the comments below.