Code Reviews and Debugging

portrait WW CookTo make applications as “bullet-proof” as possible, debugging and code reviews are necessary parts of code development.  Debugging has been around since the very first computers, while code reviews are a more recent addition. Since debugging came first, I will cover that first, followed by a section on common coding mistakes. The last section will be a brief section on code reviews.

Debugging

When an application does not work the way it is expected to work, a defect (or bug) is reported. The best way to avoid defects is to design them out from the start. Look for corner-case problems and then design and code to avoid those problems. The most common mistake is to allow an input that is out of the desired range. I will give you three examples that are quite common, along with the design work-around.

  1. Divide by zero – This error will usually cause a “core dump” because the mathematics processor will error if presented by a divide-by-zero problem. The easiest solution is to check the user input for a “0” entry for the divisor. This can be done either through an exception handler (like is often done in C#) or with a “while” loop. Please note that in C# in order to properly read more than one character from the Console, the command Console.ReadLine() must be used. in order to translate it back to a number the proper conversion must be made (i.e. int, long, double). I chose double because I wished to see the decimal portion of the number.
          // Try some division
          double dividend;
          Console.WriteLine("Enter Dividend:");
          dividend = double.Parse(Console.ReadLine());
          double divisor = 0;
          while (divisor == 0)
          {
              Console.WriteLine("Enter the divisor:");
              divisor = double.Parse(Console.ReadLine());
          }
          Console.WriteLine("The Quotient is: "+dividend/divisor);
    
  2. Entry Out of Range – For example, if you are asking the user to enter a month as a number then there needs to be a check for the number being between 1 and 12 (inclusive).
  3. C++ Library call to a Char function with a NULL pointer to a string – the NULL pointer must be checked in any char* variable in C++ because the call will cause a core dump. I created my own library routine that checked for a NULL pointer and then passed a pointer to a zero length string (“\0”) to the C++ library char* handling routine.

History of Debugging

Computer Review has an interesting article on the etymology of the words bug and debugging. Generally, the term is attributed to Rear Admiral Grace Brewster Murray Hopper working on the Harvard University’s Mark II calculator. It had relay switches with points that opened and closed. Supposedly Relay 70 stopped working and they found a moth preventing from the relay from closing. According to the story, the engineers taped the moth to the log book and then Admiral Hopper made a note about “debugging” the calculator. The only problem is that the base bug was used in words like “bugaboo” and “boogeyman” even in the early English language. these terms referred to trouble causing entities that caused weird and hard to diagnose failures.

In around 1878, according to the Yale Book of Quotes, Thomas Edison wrote a letter to Theodore Puskas stating “‘Bugs’ — as such little faults and difficulties are called — show themselves and months of intense watching, study and labor are requisite before commercial success or failure is certainly reached. ” The term “bug” was also often used in the aircraft business towards the end of World War II.

But Admiral Hopper is responsible for introducing the term to the computer industry. She often talked about the moth in relay 70 long after the incident happened. Often people put the incident happening on September 9, 1945. The only problem is that the Mark II was not installed until 1947. However, most people feel that the September 9 portion of the date is most likely accurate. By the 1950s the term was in common enough use that “everyone” understood what it meant.

Using an SDE or IDE

The benefit of using most Software Development Environments is that their editors and builders often will catch grammatical errors in your code before you can even compile them. This saves a tremendous amount of time that would be needed to debug code that has some subtle grammatical errors.

Common Mistakes

This section will be enlarged over time. Most structured languages are coded in blocks surrounded by curly braces or brackets “{” and “}”. Individual statements must be ended by a semi-colon “:” and not a Greek Question Mark “;”. The lack of a proper end-of-statement is ususally caught in the compilation stage. The common mistakes in various language types include:

Structured Languages

  1. Not ending a statement with a semi-colon “;” or ending a block “{}” with a smi-colon.
  2. Having what looks like an indented block without the curly braces around the block. In this case, only the first line will be executed as part of the block. The rest of the lines will be executed outside the block
    Bad
          while (inDivisor == 0)
              Console.WriteLine("Enter the divisor:");
              inDivisor = double.Parse(Console.ReadLine());
          Console.WriteLine("The Quotient is: " + inDividend / inDivisor);
    
    Good
          while (inDivisor == 0)
          {
              Console.WriteLine("Enter the divisor:");
              inDivisor = double.Parse(Console.ReadLine());
          }
          Console.WriteLine("The Quotient is: " + inDividend / inDivisor);
    
  3. The above example deals with missing curly braces. However, a much more serious condition, and often harder to find, is with mismatched curly braces. The code will sometimes leave the braces at the level where they were created, which will cause it to look like certain blocks are properly started and closed.

FORTRAN code

FORmula TRANslation (FORTRAN) is one of the oldest languages available. The Hollerith card is often using for storing one line at a time. Each card has 80-columns. The first six columns are for identifying a card with a label. Columns 1-5 were used for numerical labels used in other statements to identify a stamen to which to go. If column 1 had a “C”, then the entire card was a comment. Column 6 was a continuation marker. If any punch was in this card it was considered a continuation of the previous card. Columns 7-72 were for the statement itself. Columns 73-80 were unused by the program reader and were used for numbering the cards so that they could be sorted in case the program deck was ever dropped. Putting information in a wrong column would be interpreted in a way that the user would not want. This was one of the most difficult bugs to find in FORTRAN.

LISP code

In LISP the program and the data had identical forms. Pairs of parenthesis were used to program and provide data input. Output could be in any form the programmer wanted. The main problem programmers had was mismatching the parenthesis. Some people closed out the unmatched parenthesis by adding a square bracket to the end of every line to make sure that they were all closed. This is a very dangerous habit and can cause many hidden problems.

Pranks

Unfortunately, pranks are sometimes done in the coding community. If you are in a hurry they can slip by code reviews, especially when the code is printed out. Fortunately many will be caught by a good SDE.

The latest prank I saw is impossible to catch on a printed version of the code, but should be caught immediately by a good SDE. It is called the Greek Question mark paradox. The Greek question mark appears as an “;” on a print-out, but it will have a different character code and the SDE compiler should catch it and tell you what line the error is on. With the best SDE, the “;” will either be underlined or have an underline next to it to help you find the problem.

Code Reviews

Any time you can have more than two eyes looking at something (especially code), more problems or inefficiencies can be uncovered. At first, the reviews were very informal and usually consisted of just one programmer giving another programmer his/her code to review. Then came structured design, where the entire review process was formalized. The structure of these meetings were well specified, but sometimes seemed like they paid more attention to the structure than the code review itself. This led to the agile movement and various descriptions of how review meetings should be organized. For me, the SCRUM process is the best of all worlds.