I am thinking of writing a post on my blog soon to talk about the difference between code that is concise and code that is so terse that it becomes less readable or maintainable. My example of this would be from a while ago when I had to write a routine that checked if a string contained all hex characters. The terse version looked like this:

bool IsHexString(char *sz) 
{ 
        while ( isxdigit(*sz) && *++sz != NULL ) {} 
        return *sz == NULL; 
}

Much as I liked the “smallness” of my routine, in the end I coded it as follows because I thought it was more readable:

bool IsStringHex(char *sz) 
{ 
        char* ch = sz; 
        while ( *ch != NULL ) 
        { 
                if ( !isxdigit (*ch) ) 
                { 
                        return false; 
                } 
                ch++; 
        } 
        return true; 
} 

Does anyone else have a view on this? Do you have examples of routines that you have written or found that were so terse they were practically indecipherable?

It is worth mentioning that "obfuscated code" as the one in your first example runs exactly as fast and consumes exactly as much memory as the cleaner version in your second example.

The difference is HD space. The first version of the source will take up less space on your disk (that is, the source code, not the executable). Back in the dinosaur days, people had tiny disks and therefore wrote extremely compact code. So if your computer is from the mid 80s, it is motivated. Otherwise, it isn't.

:

: bool IsStringHex(char *sz) 
: { 
:         char* ch = sz; 
:         while ( *ch != NULL ) 
:         { 
:                 if ( !isxdigit (*ch) ) 
:                 { 
:                         return false; 
:                 } 
:                 ch++; 
:         } 
:         return true; 
: } 

:


Regarding the example above: you shouldn't compare with NULL. NULL is a strange and magic C constant used only for pointers. It is usually, but not necessarily, defined as

#define NULL ((void*)0)

In C++, pointers are set to 0 instead.

However, in your example you are looking for the "null termination". This should be '\0', 0 or '\x00'. The difference is that NULL may not always be 0, while null termination is always 0. To avoid mixing them up, null termination is sometimes referred to as "NUL", especially in ASCII tables.

: :

: : bool IsStringHex(char *sz) 
: : { 
: :         char* ch = sz; 
: :         while ( *ch != NULL ) 
: :         { 
: :                 if ( !isxdigit (*ch) ) 
: :                 { 
: :                         return false; 
: :                 } 
: :                 ch++; 
: :         } 
: :         return true; 
: : } 

: :


Not bad. However, why copy 'sz' into 'ch' - why not use 'sz' right in the while() loop?

Also, the contents of the 'ch' pointer (I mean *ch thing) taken twice inside the loop. You have to be sure that compiler will optimize it. I would have done it like this:

bool IsStringHex (char *sz) 
: : { 
: :         int c; 
: :         while ( (c = *sz++) != '\0' ) 
: :         { 
: :                 if ( !isxdigit (c) ) 
: :                 { 
: :                         return false; 
: :                 } 
: :         } 
: :         return true; 

From professional point of view I can say that I simply can't stand the code, which does not have any blank lines. No matter how good the code is or that it has comments, I can't help but hate this style:

// comment
code line
code line
code line
if ()
{
  code line
}
// comment
code line
code line
// comment
code line
code line
code line
// comment
code line
code line
// comment
code line

Since we are now on the sensitive topic of coding style, it is worth mentioning that there are no absolute truths. However, the vast majority of programmers dislike code with multiple operators on one line as in the first example, as well as code written without any blank lines.

Personally I dislike assignment inside the loop condition, or inside an if() statement. Because strictly, if() and while() should only evaluate boolean expressions and nothing else. Null termination and "false" both have the integer value 0, so the code will of course work, even though I wouldn't have written it like that. Avoiding assignment in conditions also eliminates any unintentional mix up between = and ==.

I would have written the function like this:

bool isStringHex (char* sz)
{
  bool result = true;

  while(*sz != '\0')
  {
    if( !isxdigit(*sz) )
    {
      result = false;
      break;
    }

    sz++;
  }

  return result;
}

: Since we are now on the sensitive topic of coding style, it is worth
: mentioning that there are no absolute truths. However, the vast
: majority of programmers dislike code with multiple operators on one
: line as in the first example, as well as code written without any
: blank lines.
:
: Personally I dislike assignment inside the loop condition, or inside
: an if() statement. Because strictly, if() and while() should only
: evaluate boolean expressions and nothing else. Null termination and
: "false" both have the integer value 0, so the code will of course
: work, even though I wouldn't have written it like that. Avoiding
: assignment in conditions also eliminates any unintentional mix up
: between = and ==.
:
: I would have written the function like this:
:
:

: bool isStringHex (char* sz)
: {
:   bool result = true;
: 
:   while(*sz != '\0')
:   {
:     if( !isxdigit(*sz) )
:     {
:       result = false;
:       break;
:     }
: 
:     sz++;
:   }
: 
:   return result;
: }

:


I agree about the assignment in statements, but only if there is only assignment. I often do that, but always together with logical operator.

Hi everyone - thank you so much for giving me your thoughts. It has taken me a long time to get around to posting about this on my blog but I finally have done in my post Terse is Worse? Once again thank you all for your input - I definitely learned something from it.