Thread: Not sure how this simple code works

Yep this is exactly why any variable that is not a counter should say what it is...pc should be something like previousCharacter, c could be currentCharacter, stuff like that, it makes the meaning of the code so much more obvious as anyone who has to maintain anothers code will tell you...
Self-documenting code. Any easier?

Code:

#include <stdio.h>
const int Success = 0;
const char Space = ' ';

/* count lines in input */
int main(int argc, char*argv[])
{
   int nReturnCode = Success;
   int currentCharacter, previousCharacter; 

   /* set previousCharacter to a value that wouldn't match any character, in case
   this program is ever modified to get rid of multiples of other
   characters */

   previousCharacter = EOF;

   while ((currentCharacter = getchar()) != EOF) 
   {
      if (currentCharacter == Space)
      {
         if (previousCharacter != Space)   /* or if (previousCharacter != c) */ 
	 {
             putchar(currentCharacter);

             /* We haven't met 'else' yet, so we have to be a little clumsy */
             if (currentCharacter != Space)
	     {
                putchar(currentCharacter);
	     }
             previousCharacter = currentCharacter;
	 }
      }
   }
   return Success;
}

Originally Posted by jeffcobb

Yep this is exactly why any variable that is not a counter should say what it is...pc should be something like previousCharacter, c could be currentCharacter, stuff like that, it makes the meaning of the code so much more obvious as anyone who has to maintain anothers code will tell you...
Self-documenting code. Any easier?

Code:

#include <stdio.h>
const int Success = 0;
const char Space = ' ';

/* count lines in input */
int main(int argc, char*argv[])
{
   int nReturnCode = Success;
   int currentCharacter, previousCharacter; 

   /* set previousCharacter to a value that wouldn't match any character, in case
   this program is ever modified to get rid of multiples of other
   characters */

   previousCharacter = EOF;

   while ((currentCharacter = getchar()) != EOF) 
   {
      if (currentCharacter == Space)
      {
         if (previousCharacter != Space)   /* or if (previousCharacter != c) */ 
	 {
             putchar(currentCharacter);

             /* We haven't met 'else' yet, so we have to be a little clumsy */
             if (currentCharacter != Space)
	     {
                putchar(currentCharacter);
	     }
             previousCharacter = currentCharacter;
	 }
      }
   }
   return Success;
}

I have to disagree. Self-documenting code would be 'prevChar' or even 'prevCh', not 'previousCharacter', which may as well be 'theQuickBrownFoxJumpedOverTheLazyDog'. You also used horrible indentation, probably due ot a mix of tabs and spaces, and more importantly, you complete broke the code. It doesn't do anything remotely close to what it supposed to any more.

Code:

const char space = ' ';

does not get you anything whatsoever.
Unused code such as

Code:

int nReturnCode = Success;

is just silly also. Dead code is a burden. The more lines of code you have, the more bugs you most likely have.
You have some good intentions with making it self-documenting, but your execution of that idea came out absolutely terrible. The originally posted code was far better.

With regards to commenting, what experience teaches you is that it is more important for comments to say WHY the code does what it does, rather than WHAT it does.

Here's how I would recommend writing this:

Code:

#include <stdio.h>

int main() {
    /* set prevch to a value that wouldn't match any character, in case
    this program is ever modified to get rid of multiples of other
    characters */

    int ch, prevCh = EOF;

    while ((ch = getchar()) != EOF) {

        /* Output the char as long a both this char
        and the last one were not both spaces */
        if (ch != ' ' || prevCh != ' ')
            putchar(ch);

        prevCh = ch;
    }

    return 0;
}

Well your brace style bites from my perspective; AFA the tabs/spaces are concerned I just copied what was there, properly braced the logic and left it be. The point of it was the variables and since the person analyzing the code was admittedly both new and lost, the most verbiage the better.

As for the const int Space and Success, I would beg to differ; at the cost of a measely few bytes (compiled) you know exactly what you are returning and more importantly what the logic is *meant* to do. True, I did not set a fail condition for the return code; that was beyond the scope but just having a return 0, you might as well not return anything at all since then the OS nor any calling program can make any determination of execution success or failure.

As for the comments, well any code-base that has been around long invariably has comments that are out of date, not kept up and so on. If functions and variables are clearly (to everyone, not the the original coder) named, the comment quality matters less and more importantly are far less to mislead the journeyman code trying to make a fix to some complicated logic.

When I was teaching this sort of thing many times meant the difference between a student getting the code and not. Sure if your more experienced like we are it doesn't matter; by now in our careers we have looked at so much dodgy code that we are used to it. The OP obviously is not.

But I do apologize for the tabs/spaces thing; it is not my want; my only excuse is that I was late for the store.