View source

This page contains a small (but growing) archive of real programming horrors. Mere poor style is not enough to get you on this page. Programs are written in C or C++ except where indicated otherwise.

In general, I don't credit the authors (lawsuits can be expensive) - if any of the authors feel that they would like me to give them a mention - just email me.

All of these are taken from real code in real applications - where possible, I have preserved the original indentation style. New examples will be added at the top of the file - so you don't have to keep searching for new ones.

== The Gentle Art Of Saying No. ==
The number of ways of misunderstanding the nature of simple boolean variables never ceases to be a source of apalling code. The simple act of negating a boolean seems to be a constant source of anguish to the incompetant programmer. We have probably all seen:

  if ( boolean )
    boolean = FALSE ;
  else
    boolean = TRUE ;

...but for variation (at least it's less typing), I have recently been given this little gem (spacing, redundant brackets and cute - but incorrect - spelling are genuine) :

  nybble_pos=(nybble_pos)? 0 : 1;

For the truly cautious (and WRONG) programmer, nothing beats:

  if ( boolean == TRUE )
    boolean = FALSE ;
  else
  if ( boolean == FALSE )
    boolean = TRUE ;

I have also been treated to:

  boolean = ~boolean ;

Which is great fun since it turns not-TRUE into something that still isn't FALSE - and for a special treat, you can sometimes spot the bug being fixed like this:

  boolean = (~boolean) & 1 ;

But <strong>please</strong>, can we stick with:

  boolean = ! boolean ;

Oh - yes! I forgot the most heinous sin of all - beloved of Pascal and BASIC programmers - and coming soon to a C program near you:

  #define TRUE   1
  #define FALSE -1     /* Eeeek! */

...oh yes, it *does* happen.
== The Worlds Least Efficient 'cast'. ==
This horror comes from a realtime clock routine - all it has
to do is to convert a number from integer picoseconds into double precision
seconds.

The code should have said something like:

   cycleval_dbl = (double) cycleval * 1e-12 ;

It actually says:

   double cycleval_dbl ;
   char temp[100];
     .
     .
     .
   sprintf (temp, "%u", cycleval);
   cycleval_dbl  = atof (temp) * 1e-12;   /* from picoseconds to seconds */

I saw this amazingly bad piece of code about two years ago, and have been amused by it ever since - however, in the course of preparing this web site, I noticed that <code>cycleval</code> is not in fact an <code>unsigned int</code> (or even an <code>int</code>) as you might expect. It is in fact an <code>int*</code> - so this code actually converts the address of some location in memory into a double!

It now becomes clear that what happened was this:
# The programmer tried using: <code>cycleval_dbl = (double) cycleval * 1e-12 ;</code> but it wouldn't compile.
# After much messing around she finally decided to bypass the error message by doing the obscene trick of using sprintf and atof to circumvent the compilers' type checking mechanism!
# The resulting code (which is part of a real-time clock reading routine) can never have been tested - because it will return totally screwy times.

== He used the 'G' word. ==
Here is one of the few remaining programmers brave enough to stand up and admit to closet <code>goto</code>-ism. The code is used for reading in the command line arguments:

          itemexpr[ 0 ] = '\0' ;
          do
          {
            strcat( itemexpr, *argv ) ;
            strcat( itemexpr, " "   ) ;
            argc-- ;
            argv++ ;
            /* otherwise argv is garbage */
            if ( argc == 0 )
              goto enditems ;
          } while ( ( *argv[ 0 ] != '-' ) ) ;
          argv-- ;
          argc++ ;
 enditems :

It looks like the programmer is trying to concatenate all the command line arguments that begin with a '-' and advance argc/argv to contain the remainder of the command line arguments.

Looking at the rest of the program, it's pretty certain that this code doesn't do what he wanted it to do - and I'd bet that this is what he really meant:

          for ( itemexpr[0] = '\0' ; argc > 0 && argv[0][0] == '-' ; argc-- )
          {
            strcat( itemexpr, *argv++ ) ;
            strcat( itemexpr,   " "   ) ;
          }

== I like typing. ==
One mark of the really heinous programmer is his evident joy of typing. Colin MacDonald (who is NOT a heinous programmer as far as I know) spotted code like this in (of all places) Nuclear Power Station monitoring code... <blink>EEEEEKKKKK!</blink>

           void write_string_overheat()
           {
             output_string[0] = 'o';
             output_string[1] = 'v';
             output_string[2] = 'e';
             output_string[3] = 'r';
             output_string[4] = 'h';
             output_string[5] = 'e';
             output_string[6] = 'a';
             output_string[7] = 't';
             output_string[8] = '\0';
             write_output_string();
           }
           void write_some_other_string() {...}
           void and_another_string() {...}
           void and_yet_another_string() {...}

== I like MUCH MORE typing. ==
This function comes from some code to figure out the infra-red emissivity of objects.  This part of the program calculates the position of the sun as a function of time of year.

The sheer stamina of it all impresses me.

The reduction of these 315 lines of code to five lines (and a couple of short lookup tables) is left as an exercise for the reader. It's amusing to note how much typing he could have saved by simply understanding the concept of an 'else' clause. Of course we know that the earth travels in an elliptical orbit around the sun, and plugging the equation of an ellipse into this function would compress it even further and improve the precision.

It's also instructive to note that the only comment in the entire function (on the last line) is completely incorrect, and the indentation scheme is "creative".

 void get_solar_dec ( solar_dec, 
                      orbital_radius, 
                      month, day, year )
 float  *orbital_radius, *solar_dec ;
 int  month, day, year ;
 {
 int  date ;
 switch ( month )
 {
  case 1:
     if ( day < 1 || day > 31 )
     {
      printf( "error: you entered a day that is not between 1 and 31\n " ) ;
      exit(1) ;
     }
     date = 0 + day ;
     break ;
  case 2:
     if ( day < 1 || day > 29 )
     {
      printf( "error: you entered a day that is not between 1 and 29\n " ) ;
      exit(1) ;
     }
     if ( day == 29 && is_leap_year( year ) == 0 )
     {
      printf( "error: this is not a leap year; february 29 does not exit\n " ) ;
      exit(1) ;
     }
     date = 31 + day ;
     break ;
  case 3:
     if ( day < 1 || day > 31 )
     {
      printf("error: you entered a day that is not between 1 and 31\n " ) ;
      exit(1) ;
     }
     date = 59 + day ;
     break ;
  case 4:
     if ( day < 1 || day > 30 )
     {
      printf("error: you entered a day that is not between 1 and 30\n " ) ;
      exit(1) ;
     }
     date = 90 + day ;
     break ;
  case 5:
     if ( day < 1 || day > 31 )
     {
      printf("error: you entered a day that is not between 1 and 31\n " ) ;
      exit(1) ;
     }
     date = 120 + day ;
     break ;
  case 6:
     if ( day < 1 || day > 30 )
     {
      printf("error: you entered a day that is not between 1 and 30\n " ) ;
      exit(1) ;
     }
     date = 151 + day ;
     break ;
   case 7:
     if ( day < 1 || day > 31 )
     {
      printf("error: you entered a day that is not between 1 and 31\n " ) ;
      exit(1) ;
     }
     date = 181 + day ;
     break ;
   case 8:
     if ( day < 1 || day > 31 ) 
     {
      printf("error: you entered a day that is not between 1 and 31\n " ) ;
      exit(1) ;
     }
     date = 212 + day ;
     break ;
   case 9:
     if ( day < 1 || day > 30 )
     {
      printf("error: you entered a day that is not between 1 and 30\n " ) ;
      exit(1) ;
     }
     date = 243 + day ;
     break ;
   case 10:
     if ( day < 1 || day > 31 )
     {
      printf("error: you entered a day that is not between 1 and 31\n " ) ;
      exit(1) ;
     }
     date = 273 + day ;
     break ;
   case 11:
     if ( day < 1 || day > 30 )
     {
      printf("error: you entered a day that is not between 1 and 30\n " ) ;
      exit(1) ;
     }
     date = 304 + day ;
     break ;
   case 12:
     if ( day < 1 || day > 31 )
     {
      printf("error: you entered a day that is not between 1 and 31\n " ) ;
      exit(1) ;
     }
     date = 334 + day ;
     break ;
   default:
     printf( "error: you entered a month that is not between 1 and 12 \n " ) ; 
     exit(1) ;
     break ;
  }
  if (is_leap_year( year ) == 1 && date > 29 ) date = date + 1;
  if ( date <= 9 )
  {
   *solar_dec = -23.28+(((23.28-22.22)/11.0)*(2.0+date)) ;
   *orbital_radius = 0.98334-(((0.98334-.98333)/11.0)*(2.0+date)) ;
  }
  if ( date > 9  &&  date <= 17 )
  {
   *solar_dec = -20.90-(((22.22-20.90)/8.)*(17.-date)) ;
   *orbital_radius = .98378-(((.98378-.98333)/8.)*(17.-date)) ;
  }
  if ( date > 17  &&  date <= 29 )
  {
   *solar_dec = -18.13-(((20.90-18.13)/12.)*(29.-date)) ;
   *orbital_radius = .98493-(((.98493-.98378)/12.)*(29.-date)) ;
  }
  if ( date > 29  &&  date <= 40 )
  {
   *solar_dec = -14.92-(((18.13-14.92)/11.)*(40.-date)) ;
   *orbital_radius = .98662-(((.98662-.98493)/11.)*(40.-date)) ;
  }
  if ( date > 40  &&  date <= 48 )
  {
   *solar_dec = -12.25-(((14.92-12.25)/8.)*(48.-date)) ;
   *orbital_radius = .98819-(((.98819-.98662)/8.)*(48.-date)) ;
  }
  if ( date > 48  &&  date <= 56 )
  {
   *solar_dec = -9.38-(((12.25-9.38)/8.)*(56.-date)) ;
   *orbital_radius = .98991-(((.98991-.98819)/8.)*(56.-date)) ;
  }
  if ( date > 56  &&  date <= 68 )
  {
   *solar_dec = -4.80-(((9.38-4.80)/12.)*(68.-date)) ;
   *orbital_radius = .99287-(((.99287-.98991)/12.)*(68.-date)) ;
  }
  if ( date > 68  &&  date <= 76 )
  {
   *solar_dec = -1.65-(((4.80-1.65)/8.)*(76.-date)) ;
   *orbital_radius = .99508-(((.99508-.99287)/8.)*(76.-date)) ;
  }
  if ( date > 76  &&  date <= 88 )
  {
   *solar_dec = 3.06-(((1.65+3.06)/12.0)*(88.0-date)) ;
   *orbital_radius = 0.99843-(((0.99843-0.99508)/12.0)*(88.0-date)) ;
  }
  if ( date > 88  &&  date <= 99 )
  {
   *solar_dec = 7.28-(((7.28-3.06)/11.)*(99.-date)) ;
   *orbital_radius = 1.00160-(((1.00160-.99843)/11.)*(99.-date)) ;
  }
  if ( date > 99  &&  date <= 107 )
  {
   *solar_dec = 10.20-(((10.20-7.28)/8.)*(107.-date)) ;
   *orbital_radius = 1.00390-(((1.00390-1.00160)/8.)*(107.-date)) ;
  }
  if ( date > 107  &&  date <= 119 )
  {
   *solar_dec = 14.22-(((14.22-10.20)/12.)*(119.-date)) ;
   *orbital_radius = 1.00708-(((1.00708-1.00390)/12.)*(119.-date)) ;
  }
  if ( date > 119  &&  date <= 129 )
  {
   *solar_dec = 17.15-(((17.15-14.22)/10.)*(129.-date)) ;
   *orbital_radius = 1.00957-(((1.00957-1.00708)/10.)*(129.-date)) ;
  }
  if ( date > 129  &&  date <= 137 )
  {
   *solar_dec = 19.15-(((19.15-17.15)/8.)*(137.-date)) ;
   *orbital_radius = 1.01138-(((1.01138-1.00957)/8.)*(137.-date)) ;
  }
  if ( date > 137  &&  date <= 149 )
  {
   *solar_dec = 21.50-(((21.50-19.15)/12.)*(149.-date)) ;
   *orbital_radius = 1.01358-(((1.01358-1.01138)/12.)*(149.-date)) ;
  }
  if ( date > 149  &&  date <= 160 )
  {
   *solar_dec = 22.87-(((22.87-21.50)/11.)*(160.-date)) ;
   *orbital_radius = 1.01518-(((1.01518-1.01358)/11.)*(160.-date)) ;
  }
  if ( date > 160  &&  date <=168 )
  {
   *solar_dec = 23.37-(((23.37-22.87)/8.)*(168.-date)) ;
   *orbital_radius = 1.01602-(((1.01602-1.01518)/8.)*(168.-date)) ;
  }
  if ( date > 168  &&  date <=172 )
  {
   *solar_dec = 23.50-(((23.50-23.37)/4.)*(172.-date)) ;
   *orbital_radius = 1.01622-(((1.01622-1.01602)/4.)*(172.-date)) ;
  }
  if ( date > 172  &&  date <= 180 )
  {
   *solar_dec = 23.50-(((23.50-23.28)/8.)*(date-172.)) ;
   *orbital_radius = 1.01662-(((1.01662-1.01622)/8.)*(180.-date)) ;
  }
  if ( date > 180  &&  date <= 190 )
  {
   *solar_dec = 23.28-(((23.28-22.47)/10.)*(date-180.)) ;
   *orbital_radius = 1.01669-(((1.01669-1.01662)/10.)*(190.-date)) ;
  }
  if ( date > 190  &&  date <= 198 )
  {
   *solar_dec = 21.35-(((22.47-21.35)/8.)*(date-198.)) ;
   *orbital_radius = 1.01639+(((1.01669-1.01639)/8.)*(198.-date)) ;
  }
  if ( date > 198  &&  date <= 210 )
  {
   *solar_dec = 18.95-(((21.35-18.95)/12.)*(date-210.)) ;
   *orbital_radius = 1.01530+(((1.01639-1.01530)/12.)*(210.-date)) ;
  }
  if ( date > 210  &&  date <= 221 )
  {
   *solar_dec = 16.10-(((18.95-16.10)/11.)*(date-221.)) ;
   *orbital_radius = 1.01384+(((1.01530-1.01384)/11.)*(221.-date)) ;
  }
  if ( date > 221  &&  date <= 229 )
  {
   *solar_dec = 13.68-(((16.10-13.68)/8.)*(date-229.)) ;
   *orbital_radius = 1.01244+(((1.01384-1.01244)/8.)*(229.-date)) ;
  }
  if ( date > 229  &&  date <= 241 )
  {
   *solar_dec = 9.65-(((13.68-9.65)/12.)*(date-241.)) ;
   *orbital_radius = 1.00986+(((1.01244-1.00986)/12.)*(241.-date)) ;
  }
  if ( date > 241  &&  date <= 252 )
  {
   *solar_dec = 5.62-(((9.65-5.62)/11.)*(date-252.)) ;
   *orbital_radius = 1.00723+(((1.00986-1.00723)/11.)*(252.-date)) ;
  }
  if ( date > 252  &&  date <= 260 )
  {
   *solar_dec = 2.57-(((5.62-2.57)/8.)*(date-260.)) ;
   *orbital_radius = 1.00510+(((1.00723-1.00510)/8.)*(260.-date)) ;
  }
  if ( date > 260  && date <= 272 )
  {
   *solar_dec = -2.10+(((2.57+2.10)/12.)*(272.-date)) ;
   *orbital_radius = 1.00170+(((1.00510-1.00170)/12.)*(272.-date)) ;
  }
  if ( date > 272  &&  date <= 282 )
  {
   *solar_dec = -5.97+(((5.97-2.10)/10.)*(282.-date)) ;
   *orbital_radius = .99888+(((1.00170-.99888)/10.)*(282.-date)) ;
  }
  if ( date > 282  &&  date <= 290 )
  {
   *solar_dec = -8.97+(((8.97-5.97)/8.)*(290.-date)) ;
   *orbital_radius = .99659+(((.99888-.99659)/8.)*(290.-date)) ;
  }
  if ( date > 290  &&  date <= 302 )
  {
   *solar_dec = -13.20+(((13.20-8.97)/12.)*(302.-date)) ;
   *orbital_radius = .99326+(((.99659-.99326)/12.)*(302.-date)) ;
  }
  if ( date > 302  &&  date <= 313 )
  {
   *solar_dec = -16.64+(((16.64-13.20)/11.)*(313.-date)) ;
   *orbital_radius = .99054+(((.99326-.99054)/11.)*(313.-date)) ;
  }
  if ( date > 313  &&  date <= 321 )
  {
   *solar_dec = -18.80+(((18.80-16.64)/8.)*(321.-date)) ;
   *orbital_radius = .98869+(((.99054-.98869)/8.)*(321.-date)) ;
  }
  if ( date > 321  &&  date <= 333 ) 
  {
   *solar_dec = -21.35+(((21.35-18.80)/12.)*(333.-date)) ;
   *orbital_radius = .98636+(((.98869-.98636)/12.)*(333.-date)) ;
  }
  if ( date > 333  &&  date <= 343 )
  {
   *solar_dec = -22.75+(((22.75-21.35)/10.)*(343.-date)) ;
   *orbital_radius = .98494+(((.98636-.98494)/10.)*(343.-date)) ;
  }
  if ( date > 343  &&  date <= 351 )
  {
   *solar_dec = -23.33+(((23.33-22.75)/8.)*(351.-date)) ;
   *orbital_radius = .98405+(((.98494-.98405)/8.)*(351.-date)) ;
  }
  if ( date > 351  &&  date <= 355 )
  {
   *solar_dec = -23.50+(((23.50-23.33)/4.)*(355.-date)) ;
   *orbital_radius = .98370+(((.98405-.98370)/4.)*(355.-date)) ;
  }
  if ( date > 355  &&  date <= 366 )
  {
   *solar_dec = -23.50+(((23.50-22.895)/10.)*(date-355.)) ;
   *orbital_radius = .98333-(((.98370-.98333)/10.)*(date-365.)) ;
  }
  }  /* end of main */

I have been told of a yet worse example of this kind of thing: A routine to sort 42 numbers into ascending order that consumes 5 megabytes of source code. However, since my MediaWiki only allows me 5Mb for this entire page, you'll have to forgo the opportunity to understand the magestic flow of that great work.  Hint: It starts off:

  if ( a > b && b < c && c < d && d < e && ....ab < ac && ac < ad && ... )
  {
    int tmp = a ;  a = b ; b = tmp ;
  }

--------------
If you have any more good examples of programming horrors,
you can email me at: [mailto:steve@sjbaker.org steve@sjbaker.org]