Not sure how this simple code works

Hi, I'm not sure how the following code works.

Code:

#include <stdio.h> /* count lines in input */ int main() { int c, pc; /* c = character, pc = previous character */ /* set pc to a value that wouldn't match any character, in case this program is ever modified to get rid of multiples of other characters */ pc = EOF; while ((c = getchar()) != EOF) { if (c == ' ') if (pc != ' ') /* or if (pc != c) */ putchar(c); /* We haven't met 'else' yet, so we have to be a little clumsy */ if (c != ' ') putchar(c); pc = c; } return 0; }

For example if I type in "John _______Smith" and press return I then get "John Smith". The spaces have been removed to just one. I've been looking through this code and disecting it for an hour and I still don't know how it does it. Any simplication of it is appreciated. Below is the original question of the solution........

"Write a program to copy its input to its output, replacing each tab by \t , each backspace by \b , and each backslash by \\ . This makes tabs and backspaces visible in an unambiguous way."

Sorry this is the original question......

Write a program to copy its input to its output, replacing each string of one or more blanks by a single blank.

There are many ways to write this piece of code but probably the best way way would to write it yourself.
You'll make mistakes but in the end you will end up learning more instead of drawing a blank pouring over someone else's.

well, this is not difficult.
take paper and pencil. write down the content of c and pc, after processing one byte after the other from the input string.

Code:

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

reads one char from the keyboard, so c contains the first char.

in words: write char to output, if its not blank.
if its blank, only write to output, if previous char wasn't blank.
move char to previous char.

it would help if u can use a debugger and watch the actual contents of each variable when stepping thru the code.

True, can't disagree with that.

Quote:

Originally Posted by kermitaner

well, this is not difficult.
take paper and pencil. write down the content of c and pc, after processing one byte after the other from the input string.

Code:

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

reads one char from the keyboard, so c contains the first char.

in words: write char to output, if its not blank.
if its blank, only write to output, if previous char wasn't blank.
move char to previous char.

it would help if u can use a debugger and watch the actual contents of each variable when stepping thru the code.

Great thanks

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; }

Quote:

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.