Bit twiddling for the win!

(I don’t recall if I wrote about this before. I really need to resurrect my old blog entries.)

This was an interesting puzzle to work on. Some of the programmers I subsequently explained it to just got that glazed look and figured I was invoking black magic. Or higher math.

Imagine you have a list of structures, each of which consists of a string key, two integer values, and some other stuff. In essence:

struct Foo
    string Name;
    int key1;
    int key2;
    // other stuff that's not relevant to this discussion

For this discussion, let’s assume that int is a 32-bit signed integer.

You have a lot of these Foo structures, more than can keep in memory, but for reasons we don’t need to get into you maintain an index that’s sorted by key1 descending, and key2 ascending. That is, in C#:

index = list.OrderByDescending(x -> x.key1)
            .ThenBy(x->x.key2);

Except that (again, for reasons we don’t need to discuss) the key has to be a single value rather than two separate values.

Packing two int values into a long is no problem. But doing it so that the result sorts with key1 descending and key2 ascending? When I first heard this proposed I wasn’t sure that it was possible. But there was a glimmer of an idea . . .

What I’m about to discuss depends on something called “two’s complement arithmetic,” which defines how most computers these days work with positive and negative integers. I’m going to illustrate with 4-bit numbers but the concepts are the same for any size integer value, including the 32-bit and 64-bit numbers we work with regularly.

With 4 bits we can express 16 possible integer values. Unsigned, that’s values 0 through 15. Signed, using the two’s complement convention, the values we can represent are -8 through +7. The table below shows that in detail.

Binary                       Binary
Value   Unsigned  Signed     Value   Unsigned  Signed
0000    0         0          1000    8         -8
0001    1         +1         1001    9         -7
0010    2         +2         1010    10        -6
0011    3         +3         1011    11        -5
0100    4         +4         1100    12        -4
0101    5         +5         1101    13        -3
0110    6         +6         1110    14        -2
0111    7         +7         1111    15        -1

When viewed as signed numbers, the high bit (the bit in the leftmost position) is called the sign bit. When it’s set to 1, the number is considered negative.

Now, the problem. We’re given two signed integers, call them key1 and key2, each of which is four bits in length. We want to combine them into a single 8-bit value so that a natural sort (i.e. sorting the single 8-bit quantity) will result in the records being ordered with key1 descending and key2 ascending.

The first thought is to just pack key1 into the high four bits, and key2 into the low four bits, and treat it as an unsigned quantity. For example, imagine you have these records:

                         Binary      Unsigned
key1 = 4, key2 = 1     0100 0001       65
key1 = 4, key2 = 7     0100 0111       71
key1 = 4, key2 = -8    0100 1000       72
key1 = 4, key2 = -1    0100 1111       79
key1 = -5, key2 = 2    1011 0010       178

Sorting as unsigned quantities, the ordering is wonky. Both key1 and key2 go from 0 to 7, then from -8 to -1.

We need to do some bit twiddling. If we flip the sign bit on key2 and then sort, the table becomes:

                         Binary      Unsigned
key1 = 4, key2 = -8    0100 0000       64
key1 = 4, key2 = -1    0100 0111       71
key1 = 4, key2 = 1     0100 1001       73
key1 = 4, key2 = 7     0100 1111       79
key1 = -5, key2 = 2    1011 1010       90

key2 is now in ascending order! That is, for records that have key1 = 4, the key2 values are in ascending order.

So how do we make key1 sort in descending order? More bit twiddling!

Consider what happens if you flip all of the bits in key1:

Decimal  Binary  Inverted  Result   Decimal  Binary Inverted  Result
  -8      1000    0111        7        0      0000    1111      -1
  -7      1001    0110        6        1      0001    1110       -2
  -6      1010    0101        5        2      0010    1101       -3
  -5      1011    0100        4        3      0011    1100       -4
  -4      1100    0011        3        4      0100    1011       -5
  -3      1101    0010        2        5      0101    1010       -6
  -2      1110    0001        1        6      0110    1001       -7
  -1      1111    0000        0        7      0111    1000       -8 

If we invert all of the bits in key1, then a natural sort orders the keys in descending order.

It’s not magic at all. Given an understanding of two’s complement arithmetic and a little study, there’s nothing to it. I won’t claim to be the first person to come up with this, but I did develop it independently.

The explanation above is for two 4-bit quantities combined into a single 8-bit quantity, but it works just as well with two 32-bit quantities combined into a 64-bit quantity.

Putting it all together, we come up with this C# function. It should be easy enough to port to any other language that allows bit twiddling:

public static long getKey(int key1, int key2)
{
    long key = (uint)~key1;  // invert all the bits of key1
    key <<= 32;

    // flip the sign bit of key2
    uint flipped = (uint)key2 ^ 0x80000000;
    key |= flipped;
    return key;
}

And of course you can write the reverse function to retrieve the two original keys from the combined key.

It really isn’t that difficult

I’ve been contributing to Stack Overflow for 14 years: pretty much ever since it started. And every year there are Computer Science students who come up with novel ways of screwing up parsing and evaluating arithmetic expressions.

It’s a problem common to all Computer Science curricula: given a string that contains an arithmetic expression (something like 2*(3+1)/4), write a program that evaluates the expression and outputs the answer. There are multiple ways to solve the problem but the easiest as far as I’m concerned is called the Shunting yard algorithm, developed by Edsger Dijkstra. It’s an elegantly simple algorithm that’s very easy to understand and almost trivial to implement once you understand it.

Here’s the funny thing. Computer Science courses often teach postfix expression evaluation (i.e. evaluating “Reverse Polish Notation” like 2 3 1 + * 4 /) as an example of how to use the stack data structure. Then they teach the Shunting Yard and show that by employing two stacks you can turn an infix expression into a postfix expression. (For example, 2*(3+1)/4 becomes 2 3 1 + * 4 /). Then they give the students an assignment that says, “Write a program to evaluate an infix expression.”

The students dutifully go home and write a program that glues the two examples together. That is, the first part converts the infix to postfix, and the second part evaluates the postfix expression to get the answer. Which works. But is kind of silly because a few simple changes to the output method for Shunting Yard let you evaluate the expression directly–without converting to postfix. In my experience, very few students figure this out.

I actually had a job interview where they asked me to write an expression evaluator. I wrote it to evaluate directly from infix, without going through the postfix step. Those white board inquisitions are always weird. At one point as I was furiously scribbling on the white board, the interviewer asked me where my postfix output was? When I told him that I wouldn’t be producing any, he insisted that I must. I don’t argue with an interviewer (more on that in a separate entry) and usually take their advice, but here I told him to just sit tight; that I knew this was going to work.

The post-coding walkthrough was fun. Usually it’s a formality in which I prove to the interviewer that the code I wrote works as intended. The interviewer already knows whether it works or not, because he’s seen the same algorithm written dozens of different times, knows all the different ways it can be screwed up, and knows what to expect from the walkthrough. But this time the interviewer was following along intently as I explained to him how it all worked. A senior software developer at a major company had never before seen the Shunting yard implemented without that unnecessary conversion to postfix. Weird.

I wonder why this oddity exists. Do Computer Science instructors not tell their students that there’s a way to do the evaluation directly? Or do the instructors themselves not know? And then there’s the other question: why would legions of C.S. students every year come to Stack Overflow looking for the answer to a question that’s easily answered with a simple Google search?

Well, okay, I have an answer to the last one. Laziness. It’s easier to post a question asking, “What’s wrong with my code?” than it is to understand how what the code actually does differs from what it’s supposed to be doing.

But that’s the way we’ve always done it!

Some years back I did a lot of research on and experimentation with the binary heap data structure. During that time I noticed that publicly available implementations placed the root of the heap at array[1], even in languages where arrays are 0-based. I found that incredibly annoying and wondered why they did that.

As far as I’ve been able to determine, the binary heap data structure originated with Robert J. Floyd in his 1962 article, “TreeSort” (Algorithm 113) in the August 1962 edition of Communications of the ACM. As with all algorithms published in CACM at the time, it was expressed in Algol: a language with 1-based arrays. The calculation to find the left child of any parent is: childIndex = parentIndex * 2. The right child is (parentIndex * 2) + 1, and the parent of any node can be found by dividing the node index by 2.

When C gained popularity as a teaching language in the 1980s (for reasons that escape me, but I digress), authors of algorithms texts converted their code from Pascal (again, 1-based arrays) to C (0-based arrays). And instead of taking the time to modify the index calculations, whoever converted the code made a terrible decision: keep the 1-based indexing and allocate an extra element in the array.

It’s almost certain that the person who converted the code was smart enough to change the index calculations. I think the reason they didn’t was laziness: they’d have to change the code and likely also change the text to match the code. So they took the easy route. And made a mess.

Programmers in C-derived languages (C++, Java, C#, JavaScript, Python, etc.) learn early on to start counting at 0 because arrays start at 0. If you want an array that holds 100 items, you allocate it: int a[100];, and the indexes are from 0 through 99. This is ingrained in our brains very early on. And then comes this one data structure, the binary heap, in which, if you want to hold 100 items, you have to allocate 101! The element at index 0 isn’t used.

That’s confusing, especially to beginning programmers who are the primary consumers of those algorithms texts.

Some argue that placing the root at array[1] makes the child and parent calculations faster. And that’s likely true. In a 0-based heap, the left child node index is at (parentIndex * 2) + 1, and the parent index of any child is (childIndex - 1)/2. There’s an extra addition when calculating the left child index, and an extra subtraction when calculating the parent. The funny thing is that in Algol, Pascal, and other languages with 1-based arrays, those “extra” operations existed but were hidden. The compiler inserted them when converting the 1-based indexing of the language to the 0-based indexing of the computer.

But in the larger context of what a binary heap does, those two instructions will make almost no difference in the running time. I no longer have my notes from when I made the measurements myself, but as I recall the results were noisy. The difference was so small as to be insignificant. Certainly they weren’t large enough in my opinion to justify replacing the code. If my program’s performance is gated on the performance of my binary heap, then I’ll replace the binary heap with a different type of priority queue algorithm. A paring heap, perhaps, or a skip list. The few nanoseconds potentially saved by starting my heap at index 1 just isn’t worth making the change.

The C++ STL priority_queue, the Java PriorityQueue, the .NET PriorityQueue, and python’s heapq all use 0-based binary heaps. The people who wrote those packages understand performance considerations. If there was a significant performance gain to going with a 1-based binary heap, they would have done so. That they went with 0-based heaps should tell you that any performance gain from a 1-based heap is illusory.

I am appalled that well-known authors of introductory algorithm texts have universally made this error. They should be the first to insist on clear code that embraces the conventions of the languages in which they present their code. They should be ashamed of themselves for continuing to perpetrate this abomination.

First, prove that it’s possible

Building on my previous post about finding a working solution first, before thinking of optimization issues.

I have been fortunate to work on many interesting problems in my career, and I have used that approach every time: first, find something that works. Maybe it works with a subset of the actual data, or solves the problem for specific cases. Maybe the initial solution takes two full days to come up with a solution using only 0.1% of the data. Or perhaps it’s only a partial solution.

That’s okay! As they say, Rome wasn’t built in a day.

Software development involves a certain amount of discovery: figuring out how to do things. The first part of figuring out how to do something is determining if it’s even possible. You need an existence proof. Once you have an existence proof, everything else is just optimization.

Don’t laugh. I’m not trying to minimize the amount of work that might be involved in optimizing a general solution. Rather, I’m saying that no amount of optimization can possibly help if you don’t know whether a solution is possible.

As a senior developer, I’ve dealt with this too many times. Management decides they want a new feature and immediately builds a team to start implementing it, not stopping to ask themselves whether a solution is possible. In one memorable case the problem they wanted to solve was essentially a 10,000 node traveling salesman problem to which they wanted the optimum solution. They built a team with a half dozen programmers and set them to work building the backend infrastructure and asked me to lead the development effort. It didn’t take me very long to determine that what they wanted to do was impossible. It took months to convince them that we could provide a “good enough” solution (at a huge cost, I might add), but that we couldn’t provide a guaranteed optimum solution at any price.

Programmers often aren’t any better. Faced with a problem, many programmers immediately begin searching their brains for all the optimization tricks they’ve learned over the years. That’s all well and good, except they often forget the first step: is a solution even possible.

Upon reflection, that is one of the key differences between a computer science curriculum and a programming job. An undergraduate computer science curriculum teaches one how to solve problems that have known solutions. The basic assumption behind every assignment is that a solution does in fact exist. The student’s task is to discover and implement that solution. But in industry we are often faced with problems for which there is no known solution. We have to determine whether a solution is even possible.

I’ll admit that, even now, I usually approach new problems with the assumption that a solution is possible. But, having been around the block a time or three, I’m prepared for the possibility that a solution is not possible, or not possible given the constraints under which I am working. That’s where things get really interesting. Anybody can write a program to find “the answer.” It takes an entirely different set of skills to write a program that selects the “best” or “good enough” answer within the constraints of time and budget.

Find a solution first

I’ve mentioned before that I contribute to answering questions on StackOverflow. Something I see all too often is a question asking for “the most efficient way” to solve a first- or second-semester homework problem. It’s clear from the content of the question that the person asking has absolutely no idea how to approach finding a solution.

The best advice I ever got about computer programming, and I was fortunate to get this advice very early on, was essentially “Find a solution. Any solution. Prove that it works. Then find a more efficient solution.” I got that advice 45 years ago and it’s never failed me. In fact, the times I’ve ignored that advice and gone straight to looking for an efficient solution have been some of the most difficult.

There are two primary benefits to finding a simple (non-optimum) solution:

  1. You gain insight into the problem by observing how your solution works.
  2. You have a known-good solution against which you can test any optimized solution.

Don’t discount the power of those benefits, especially in entry-level computer science work. The assignment is, essentially, “solve the problem.” A simple solution fulfills the goals of the assignment. Do that first! Then, if you’re curious (or can get extra points), spend some time looking for a more efficient method.

This advice helps not only in school, but also once you get out in the workforce. Very often you will be tasked with solving a problem like a data conversion: a one-time job. The simple solution might take a day’s worth of effort, combined manual work and computer processing. Or you could spend a week developing an optimized solution that will convert the data in an hour of processing time. Which do you choose? I can tell you up front that your employer will be much happier if you get it done tomorrow rather than next week.

Sometimes the non-optimum solution is good enough. Implement it and move on.

You can write a working program and then spend a bit more time making it fast. Or, you can write a fast program and spend the rest of your career trying to make it work.

Responding to errors at Amazon

Saying that Amazon’s servers handle a lot of traffic is an understatement. Publicly available information estimates something like 180 million unique visitors per month in the US. Amazon reported over seven million orders on Black Friday 2017. The amount of traffic we process is just staggering, and the complexity of the underlying infrastructure is mind boggling.

When errors occur, and they do, distressingly often, we follow a four-step process:

  1. Identification. Figure out what the heck is going on. Understand it well enough so that you can move on to:
  2. Mitigation. Stabilize the service so that the error no longer occurs. This might take the form of increasing capacity, rolling back a recent change, making an emergency patch, throttling a noisy client service, etc.
  3. Correction. Modify code or processes to prevent that specific error from occurring again.
  4. Understanding. Study the event more deeply to understand how our processes allowed that error to occur, and how we can improve our processes to prevent similar errors in the future. The output from this step is a Correction of Errors document, or COE.

The first three steps are pretty standard fare: things you would expect any team that is running mission-critical services to do. The last, though, is surprisingly rare in the industry. Some companies say that they do postmortem investigations and such, but often it’s just lip service. More ominously, many times that investigation is an exercise in assigning blame for the error as a prerequisite to disciplinary action. Nobody wants to make a mistake because somebody will end up being blamed, and possibly fired. Error investigation becomes an exercise in CYA.

The COE process at Amazon is different. The purpose really is to dive deep so that we fully understand how we made the error, and how we can prevent making the same type of error in the future. The guidelines for writing a COE specifically discourage identifying individuals by name. The format is well specified. It seeks to answer the following questions:

  1. What happened?
  2. How did it affect customers?
  3. How did you identify the error?
  4. How did you fix the error, once identified.
  5. Why did the error occur?
  6. What did you learn from this incident?
  7. What will you do to prevent this from happening again?

These are hard questions to answer. The “why” section is especially difficult because you have to keep asking “why” until you get to the root cause of the problem. It’s much harder than it seems at first look.

This process of investigation to find the root cause has spawned a new verb phrase: “to root cause.” As in “I need to root cause that problem.” That drives me bonkers. I refuse to use that particular jargon, and I’ve been known to express my displeasure at its use. I fear, though, that I’m fighting a battle that’s already been lost.

COE reviews can be uncomfortable because the reviewers are very sharp and adept at asking probing questions. A review can be especially uncomfortable if the reviewers detect that the investigation wasn’t thorough, or that essential information was omitted. Trying to hide mistakes or deflect responsibility can get one in trouble. On the other hand, a thorough investigation followed by a COE that shows a deep understanding of the error and meaningful action items to prevent similar occurrences can reflect very positively on the person or people involved.

Writing a COE is, fundamentally, an exercise in taking ownership of one’s mistakes, learning from them, and passing that knowledge on to others.

Amazon understands that errors happen. One of our Leadership Principles is “Bias for Action,” which advocates calculated risk taking. Nevertheless, errors can be costly. The COE process is an attempt to gain something positive from those occurrences so that we can avoid having to pay for them again. The COE document is an opportunity for the people involved to demonstrate other Leadership Principles: ownership, their ability to dive deep and understand things at a fundamental level, and their insistence on the highest standards.

In a very real sense, making a mistake can be a positive thing for the company and for your career. I’m not saying that you should strive to make mistakes (after all, another Leadership Principle is that leaders “Are right, a lot”), but responding positively to the occasional mistake can be a Good Thing.

How did this happen?

Last time I showed two different implementations of the naive method for generating unique coupon codes. The traditional method does this:

    do
    {
        id = pick random number
    } until id not in used numbers
    code = generate code from id

The other is slightly different:

    do
    {
        id = pick random number
        code = generate code from id
    } until code not in used codes

Before we start, understand that I strongly discourage you from actually implementing such code. The naive selection algorithm performs very poorly as the number of items you choose increases. But the seemingly small difference in these two implementations allows me to illustrate why I think that the second version is broken in a fundamental way.

Think about what we’re doing here. We’re obtaining a number by which we are going to identify something. Then we’re generating a string to express that number. It just so happens that in the code I’ve been discussing these past few entries, the expression is a six-digit, base-31 value.

So why are we saving the display version of the number? If we were going to display the number in decimal, there’d be no question: we’d save the binary value. After all, the user interface already knows how to display a binary value in decimal. Even if we were going to display the number in hexadecimal, octal, or binary, we’d store the binary value. We certainly wouldn’t store the string “3735928559”, “11011110101011011011111011101111”, or “DEADBEEF”. So, again, why store the string “26BB62” instead of the the 32-bit value 33,554,432?

I can’t give you a good reason to do that. I can, however, give you reasons not to.

  1. Storing a six-character string in the database takes more space than storing a 32-bit integer.
  2. Everything you do with with a six-character code string takes longer than if you were working with an integer.
  3. Display logic should be part of the user interface rather than part of the database schema.
  4. Changing your coding scheme becomes much more difficult.

The only argument I can come up with for storing codes rather than integer identifiers is that with the former, there’s no conversion necessary once the code is generated. Whereas that’s true, it doesn’t hold up under scrutiny. Again, nobody would store a binary string just because the user interface wants to display the number in binary. It’s a simple number base conversion, for crying out loud!

If you’re generating unique integer values for database objects, then let the database treat them as integers. Let everybody treat them as integers. Except the users. “26BB62” is a whole lot easier to read and to type than is “33554432”.

I’ll be charitable and say that whoever came up with the idea of storing the display representation was inexperienced. I’m much less forgiving of the person who approved the design. The combination of the naive selection algorithm, storing the generated coupon code rather than the corresponding integer, and the broken base conversion function smacks of an inexperienced programmer working without adult supervision. Or perhaps an inexperienced programmer working with incompetent adult supervision, which amounts to the same thing.

The mere existence of the naive selection algorithm in this code is a sure sign that whoever wrote the code either never studied computer science, or slept through the part of his algorithms class where they studied random selection and the birthday paradox (also, birthday problem). The comment, “There is a tiny chance that a generated code may collide with an existing code,” tells us all we need to know of how much whoever implemented the code understood about the asymptotic behavior of this algorithm. Unless your definition of “tiny” includes a 10% chance after only one percent of the codes have been generated.

The decision to store the generated code string surprises me. You’d think that a DBA would want to conserve space, processor cycles, and data transfer size. Certainly every DBA I’ve ever worked with would prefer to store and index a 32-bit integer rather than a six-character string.

The way in which the base conversion function is broken is hard evidence that whoever modified it was definitely not an experienced programmer. I suspect strongly that the person who wrote the original function knew what he was doing, but whoever came along later and modified it was totally out of his depth. That’s the only way I can explain how the function ended up the way it did.

Finally, that all this was implemented on the database looks to me like a case of seeing the world through database-colored glasses. Sure, one can implement an obfuscated unique key generator in T-SQL. Whether one should is another matter entirely. Whoever did it in this case shouldn’t have tried, because he wasn’t up to the task.

However this design came to exist, it should not have been approved. If there was any oversight at all, it should have been rejected. That it was implemented in the first place is surprising. That it was approved and put into production borders on the criminal. Either there was no oversight, or the person reviewing the implementation was totally incompetent.

With the code buried three layers deep in a database stored procedure, it was pretty safe from prying eyes. So the bug remained hidden for a couple of years. Until a crisis occurred: a duplicate code was generated. Yes, I learned that the original implementation didn’t even take into account the chance of a duplicate. Apparently somebody figured that with 887 million possible codes, the chance of getting a duplicate in the first few million was so remote as to be ignored. Little did they know that the chance of getting a duplicate within the first 35,000 codes is 50%.

I also happen to know that at this point there was oversight by a senior programmer who did understand the asymptotic behavior of the naive algorithm, and yet approved “fixing” the problem by implementing the duplicate check. He selected that quick fix rather than replacing the naive algorithm with the more robust algorithm that was proposed: one that does not suffer from the same pathological behavior. The combination of arrogance and stupidity involved in that decision boggles the mind.

The history of this bug is rooted in company culture, where the database is considered sacrosanct: the domain of DBAs, no programmers allowed, thank you very much. And although programmers have access to all of the source code, they aren’t exactly encouraged to look at parts of the system outside of their own areas. Worse, they’re actively discouraged from making suggestions for improvement.

In such an environment, it’s little surprise that the horribly broken unique key generation algorithm survives.

So much for that. Next time we’ll start looking at good ways to generate unique, “random-looking” keys.

Tales from the trenches: It’s just a warning

Back in the bad ol’ days when we were writing Windows programs in C and directly calling the Windows API, the compiler would produce many warnings about one’s code. Conventional wisdom was to ignore the warnings. “If it was a real problem, the compiler would have issued an error.”

The idea behind compiler warnings was that they pointed out potential programming errors. For example, due to integer overflow, a conversion from unsigned integer to signed integer could be a problem if the unsigned int were large enough. A single compile might produce thousands of warnings, most of which weren’t actual problems. As a result, compiler warnings were treated like The Boy Who Cried Wolf.

But not all warnings are alike, and even some of those unsigned-to-signed warnings really were important. One day I realized that the compiler had produced warnings for some of the bugs that I’d been encountering, and I began writing my code to avoid the warnings. Surprisingly enough, I began to see fewer bugs in my programs.

Some time after I’d had my epiphany, I got a contract to rescue a failing project. The program was “feature complete,” but incredibly unstable. There was a huge number of bugs, and the project was falling further behind schedule. The team had experienced a lot of turnover, including the original lead and several of the senior developers, and they couldn’t give a reliable estimate of when they’d have the program working.

The first day I was there, I got my development system set up and compiled the program. As expected, there were over 1,000 compiler warnings. I spent a few hours looking through the code in question and found a half dozen warnings that were pointing out real problems with the code, and proved that modifying the code to eliminate the warnings resolved some reported bugs. The next day I met with the development team.

If you ever wrote Windows programs back then, you can imagine the response to my proclamation: “The first thing we’re going to do is eliminate all of the compiler warnings.”

“But there’s more than 1,000 warnings!”

“That will take us forever!”

And, the one I was waiting for: “They’re just warnings. If it was important the compiler would have issued an error.”

At that point, I trotted out my half dozen examples of warnings that pointed to actual program bugs. That got everybody’s attention.

It took us a week to eliminate those thousand-plus compiler warnings. In the process, we resolved dozens of reported bugs that were directly attributable to the warnings. By the end of the week the development team was in good spirits and the project manager thought I was a miracle worker.

The following Monday I met with the development team and said, “Now we’re going to Warning Level four.”

And I thought the previous week’s protests were bad! I had compiled the program on Level 4 and got over 2,000 warnings. Many of those truly are spurious: unreachable code, unused parameters and local variables, etc. But some of those warnings are important. For example, the compiler would issue a warning if the code used a variable before it was initialized. I had found a bug in the program that was caused by referencing an uninitialized pointer, but that warning only occurs on Level 4.

It took almost two weeks, and the development manager had to “counsel” one programmer who, rather than fixing the offending code, was instructing the compiler not to issue the warnings.

Not surprisingly, we resolved another few dozen bugs in the process and the code was a lot cleaner, too. A month after I started, we had a solid schedule to fix the remaining issues, and the product shipped six weeks later. The development team was happy and the manager thought I was a miracle worker. My contract had been for three months (13 weeks) but they let me go after 10 weeks, with an extra week’s pay as a bonus. To top things off, the manager set me up with a friend of his who also had a troubled project.

It was a good gig for a while, getting paid for making people pay attention to what their compilers were telling them. Good pay, easy work, and people thinking I was some kind of genius. It’s a weird world we live in.

The moral of the story, of course, is that you shouldn’t ignore compiler warnings. A more important lesson, which the C# team took to heart, is that compiler warnings are ambiguous. The C# language specification is much more rigorous than is the C language specification, and most things that were “just warnings” in C are definite errors in C#. For example, referencing an uninitialized variable is an error. There are fewer than 30 compiler warnings in the latest version (Visual Studio 2015) of the C# compiler. Contrast that to the hundreds of different warnings in a C or C++ compiler.

In my work, I compile with “Warnings as errors.” If the compiler thinks something is important enough to warn me about it, then I’m going to fix the code so that the compiler is happy with it. It’s usually an easy fix. I almost never have to disable the warning altogether.

Null is evil, Chapter 9346

If we’ve learned anything from 45 years of C coding, it’s that NULL is evil. At least relying on NULL return values is usually evil, especially when it forces you to write contorted special case handling code. I ran across a case in point today that surprised me because the programmer who wrote it should have known better. And for sure the senior programmer who reviewed the code should have flagged it for rewriting.

There is a method that takes an existing object and updates it based on fields in another object. The code writes a change log entry for each field updated. The code is similar to this:


    private bool UpdatePerson(Person person, Models.Person personUpdate)
    {
        var changes = new List<ChangeLogEntry>();
        
        changes.Add(CompareValues(person.FirstName, personUpdate.FirstName, "First Name"));
        changes.Add(CompareValues(person.LastName, personUpdate.LastName, "Last Name"));
        // ... other fields here
        
        changes = changes.Where(x => x != null).ToList();  // 

The CompareValues function returns a ChangeLogEntry if the field has changed. If the field didn’t change, CompareValues returns null.

This is Just Wrong. It works, but it’s wonky. The code is adding null values to a list, knowing that later they’ll have to be removed. Wouldn’t it make more sense not to insert the null values in the first place?

One way to fix the code would be to write a bunch of conditional statements:


    ChangeLogEntry temp;
    temp = CompareValues(person.FirstName, personUpdate.FirstName, "First Name");
    if (temp != null) changes.Add(temp);
    
    temp = CompareValues(person.LastName, personUpdate.LastName, "Last Name");
    if (temp != null) changes.Add(temp);
    
    // etc., etc.

That’s kind of ugly, but at least it doesn’t write null values to the list.

The real problem is the CompareValues function:


    private ChangeLogEntry CompareValues(string oldValue, string newValue, string columnName)
    {
        if (oldValue != newValue)
        {
            var change = new ChangeLogEntry(columnName, oldValue, newValue);

            return change;
        }

        return null;
    }

Yes, it’s poorly named. The method compares the values and, if the values are different, returns a ChangeLogEntry instance. If the values are identical, it returns null. Perhaps that’s the worst thing of all: returning null to indicate that the items are the same. This model of returning null to indicate status is a really bad idea. It forces clients to do wonky things. Of all the horrible practices enshrined in 40+ years of C-style coding, this one is probably the worst. I’ve seen and had to fix (and, yes, written) too much terrible old-style C code to ever advocate this design pattern. null is evil. Avoid it when you can.

And you usually can. In this case it’s easy enough. First, let’s eliminate that CompareValues function:


    if (person.FirstName != personUpdate.FirstName) changes.Add(new ChangeLogEntry(person.FirstName, personUpdate.FirstName, "First Name"));
    if (person.LastName != personUpdate.LastName) changes.Add(new ChangeLogEntry(person.LastName, personUpdate.LastName, "Last Name"));

That’s still kind of ugly, but it eliminates the null values altogether. And we can create an anonymous method that does the comparison and update, thereby removing duplicated code. The result is:


    private void UpdatePerson(Person person, Models.Person personUpdate)
    {
        var changes = new List<ChangeLogEntry>();
        
        // Anonymous method logs changes.
        var logIfChanged = new Action<string, string, string>((oldValue, newValue, columnName) =>
        {
            if (oldValue != newValue)
            {
                changes.Add(new ChangeLogEntry(columnName, oldValue, newValue));
            }
        }
        
        logIfChanged(person.FirstName, personUpdate.FirstName, "First Name");
        logIfChanged(person.LastName, personUpdate.LastName, "Last Name");
        // ... other fields here

        if (changes.Any())
        {
            // update record and write change log
        }
    }

That looks a lot cleaner to me because there are no null values anywhere, and no need to clean trash out of the list.

Here’s the kicker: I didn’t rewrite the code. If the code were as simple as what I presented here, I probably would have changed it. But the code is a bit more complex and it’s currently running in a production environment. I hesitate to change working code, even when its ugliness causes me great mental pain. I’m almost certain that I can make the required changes without negatively affecting the system, but I’ve been burned by that “almost” too many times with this code base. If I had a unit test for this code, I’d change it and depend on the test to tell me if I screwed up. But there’s no unit test for this code because … well, it doesn’t matter why. Let’s just say that this project I inherited has a number of “issues.” The one I’ll relate next time will boggle your mind.

Leadership and development teams

When he can, a leader will explain to his subordinates the reasons for his orders. Not because he has to, but because he knows that people usually do a better job when they know why they’re doing it. In addition, the leader’s subordinates are then willing to follow orders that the leader can’t explain (usually due to time pressure or enforced secrecy) because they trust that he has good reasons.

The leader earns obedience through mutual trust and respect.

Leadership is about building and directing a team: a group of people who trust and respect each other, and work together towards a common goal. It’s also about giving team members the information and authority they need to get the job done, and then letting them do it. The person who insists on being involved in the minutiae of every team members’ job is no leader at all, but rather a meddler who kills morale and destroys the team’s performance.

The best leaders are those who appear to do nothing at all, but behind the scenes they’re getting the team members the information and access they need, and are clearing obstacles from the team’s path before the rest of the team members even know those obstacles exist.

A well-lead team can function without the leader for a surprisingly long time. If the leader has done his job, then his team’s way has been cleared of any looming obstacles, they have everything they need to complete their current tasks, and a good idea of what they’ll be doing next.

In short: the leader doesn’t do the work; he makes it possible for his team to do the work.

The above is as true for a programming team lead as it is for the leader of a military unit or a senior manager of a Fortune 500 company. The team’s jobs might be completely different, but the team leader’s job is essentially the same: create an environment that makes it possible for the team to get the job done, and then step back. Observe. Give encouragement and direction when necessary. But let the team members do the things they’ve been hired to do.

It’s been my experience that most software development teams do not have good leaders. It shows in missed release dates, high bug counts, frequent “hotfix” releases, team member dissatisfaction and flouting of imposed standards, and high levels of past-due technical debt. The result is bloated, crufty, unstable code characterized by tight coupling, low cohesion, quick fixes, special cases, and convoluted dependencies. All to the detriment of the product.

All too often, the “leader” of the team has upper management convinced that the type of software his team is building is somehow different, and those problems are unavoidable in the company’s particular case. He probably believes it himself. As long as management accepts that, they will continue to experience missed delivery dates and their users will continue to be unhappy.