K&R Exercise 1-9: output the input, replacing multiple blanks by a single blank

Question

I've been working through some books on C trying to get my C legs (sea-legs! Get it?!). I've just finished exercise 1-9 from the K&R book, which for reference is to "write a program to copy its input to its output, replacing each string of one or more blanks by a single blank." I have a question about what's going on with my code, though--

#include <stdio.h>

//Copy input to output. Replace each string of multiple spaces with one single space

int main(int argc, char *argv[]){

    int ch, lch;      // Variables to hold the current and last characters, respectively


    /* This loop should 'put' the current char, then store the current char in lc,
     * loop back, 'get' a new char and check if current and previous chars are both spaces.
     * If both are spaces, do nothing. Otherwise, 'put' the current char
     */

    for(ch = getchar(); (ch = getchar()) != EOF; lch = ch){
            if(ch == ' ' && lch == ' ')
                    ;
            else putchar(ch);
    }

    return 0;
}

This mostly works, except for the very first character input. For instance, if the first line input is

"This        is   a test"

my code outputs

"his is a test". 

After dropping the very first character input, the program works consistently to meet the exercise's demands.

Can someone give me an idea of the mistake I made in my loop that's causing the issue? Any other advice is welcome as well.

Answer 1

In the for-loop statement, you're having the bug.

for(ch = getchar(); (ch = getchar()) != EOF; lch = ch){...}

Here, you're storing first character in ch, and then again testing if (ch!=EOF) by again reading character input.

Remove ch=getchar() from the initialisation statement; let it be in the second part.

for(;(ch = getchar()) != EOF; lch = ch){...}

Also, you will have to initialise your lch before making it run as lch will not have any value stored in it before making comparison in the first iteration of the loop. So, let lch=0 be initialised first.

for(lch = 0; (ch = getchar()) != EOF; lch = ch){...}

Consider enabling warnings in your compiler, it would probably detect and warn about this issue, so you could fix it.

The above would solve your problem.

(Thanks to Blue Moon and hyde for helping me modify the answer.)

Answer 2

You call getchar twice in the loop initialization:

 for(ch = getchar(); (ch = getchar()) != EOF; lch = ch)

Instead, you should call it once in the initialization (to get the first char) and then in the end of the iteration (to get the next characters):

int ch, lch = 0; // avoid using uninitialized variable

for(ch = getchar(); ch != EOF; lch = ch)
{
        if(ch == ' ' && lch == ' ')
                ;
        else putchar(ch);

        ch = getchar();
} 

UPD: Thanks Blue Moon and shekhar suman for pointing out the issue with lch

Answer 3

The problem is that the first iteration of your loop calls getchar twice - once when initializing the ch variable, and one more time when checking ch against EOF.

Dropping ch = getchar() will fix this problem:

for( lch = '?' ; (ch = getchar()) != EOF; lch = ch) {
    ...
}

Note that you need to init lch with any value other than space.

Answer 4

You are calling getchar() once before the loop starts, then once per iteration in the for condition. The first character you retrieve is thus discarded.

You also need to initialize lch before looping, before comparing it. Depending on what you want to do when the first character of your string is a space :

• Setting it to ' ' will trim leading space by "pre-matching" it.
• Setting it to anything else will treat leading space normally.

Your loop header becomes (in the second case) :

 for(lch = 'a' /*arbitrary*/; (ch = getchar()) != EOF; lch = ch)

Thanks to shekar suman for the heads-up about the uninitialized lch.

Answer 5

Change this loop

for(ch = getchar(); (ch = getchar()) != EOF; lch = ch){
        if(ch == ' ' && lch == ' ')
                ;
        else putchar(ch);
}

the following way

for( lch = EOF; ( ch = getchar() ) != EOF; lch = ch )
{
        if ( ch != ' ' || lch != ' ' ) putchar( ch );
}

Otherwise in the begining of the loop you read a character twice.

Also in my opinion the assignment describes another task

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

You should replace each whole line of blanks with a single blank.:) The loop shown above does not do this task.

Answer 6

Unless the task is to do it with a for-loop, it is better for learnig the language if you try to get cleaner code. Just tell yourself what the code does, compare for example the equivalent while-loop with the for-loop:

//initialize lch to prevent undefined behaviour
//if the first character is a space, it will be printed
lch = 'A';

// as long as you can read characters
while((ch = getchar()) != EOF) {

    // if either the current character or the previous one is not a space
    if(ch!=' ' || lch!=' ') { 

        //print it
        putchar(ch);
    }

    // remember the current for the next round
    lch = ch;
}

Once you understand the while-construct, you can also convert it to the hacky for-loop, but why would you? The while is easier to read and the compiler doesn't care because it will compile both the same way. (probably)

Answer 7

Although there are plenty of correct answers, let me give you a hint how you could have tracked this down yourself by using a debugger (gdb here):

First change the code to look like this (one statement per line only!):

...

for(ch = getchar(); 
   (ch = getchar()) != EOF; 
   lch = ch){

...

Now compile it using symbols (-g for gcc), then run the code using a debugger:

 gdb ./a.out

Put a break point at main():

(gdb) break main

Start the program:

(gdb) run

See it stopping on main():

Breakpoint 1, main (argc=1, argv=0x7fffffffe448) at main.c:15
15      for(ch = getchar(); 
(gdb) 

Step through the code:

(gdb) step

Use print ch from gbd command line to inspect interesting variables (ch here) at various stages of the "running" code, while stepping through it.

More details on how to steer gbd here: http://beej.us/guide/bggdb/

Answer 8

Yes, what´s hapening is that when you are declaring your for statement, first you initialize the ch with

for( ch= getchar();

So at this moment you get your first char (T) and the pointer advances one position to the next char (h)

then you get again the char with (ch = getchar()) !=EOF;

try changing for (ch= getchar(); and use for (ch= '' ; instead.

Hope that fixes it.

Answer 9

There are three parts to a for statement: initialization, condition, and increment. These parts are separated by the two semicolons.

It's very confusing when the condition part of a for statement has side effects. Side effects belong in the increment part:

for (ch = getchar(); ch != EOF; lch = ch, ch = getchar())

And, as others have indicated, lch has to be initialized, so:

int lch = 'a';

And, finally, although this doesn't affect the correctness of the program, I'd reverse the if test:

if (ch != ' ' || lch != ' ')
    putchar(ch);

Answer 10

This worked for me

#include <stdio.h>
int main(int arg, char *argv[]){
char c = 0;
long blank = 0;
long tab   = 0;
while((c=getchar())!= EOF){
 if(c == ' '){
    ++blank;  
 }

 if(c != ' '){
     if(blank>1){
       printf("%c", ' ');
       blank = 0;
       printf("%c", c);            
        }
 else{
        printf("%c", c);                            
     }
  }    

 } //end of while
return 0;
}

Answer 11

There was a minor change from @elessar. Line 12 had to be changed from (blank>1) to (blank>=1) because the previous one would not print single blanks.

#include <stdio.h>
int main(int arg, char *argv[]){
char c = 0;
long blank = 0;
long tab   = 0;
while((c=getchar())!= EOF){
 if(c == ' '){
    ++blank;  
 }

 if(c != ' '){
     if(blank>=1){
       printf("%c", ' ');
       blank = 0;
       printf("%c", c);            
        }
 else{
        printf("%c", c);                            
     }
  }    

 } //end of while
return 0;
}

Answer 12

Another Abordation:

#include <stdio.h>

int main()
{
    int charac;

    // Variable declared for verifying consecutive whitespaces 
    bool blank = false;

    // As long as you did not input EOF (Ctrl + Z on Windows, Ctrl + D on linux, macOS)
    while ((charac = getchar()) != EOF){

        // Current char is whitespace, the one before was also whitespace => go to next iteration    
        if((charac == ' ') && (blank == true)){
            continue;
        }
        // If current char is whitespace, keep this in mind(blank = true) and output the whitespace
        else if(charac == ' ')
        {
            blank = true;
            putchar(charac);
            continue;
        }

        // If current character is not whitespace, output it and reset the blank boolean
        putchar(charac);
        blank = false;
    }

    return 0;
}

Answer 13

I'm also reading this book learning C and I managed to come up with this approach, I would like some feedback to improve. I tried not to declare too many variables in order not to waste memory space. I ended up defining the blanket space to print it later because I wanted to treat the multiple tabs and spaces as one case.

#include <stdio.h>

/* space char was defined so I can treat ' ' and '\t' on the same case */
#define BLANK ' '


int main(){

    int c;

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

        /* if char is either ' ' or '\t' */
        if((c == ' ') || (c == '\t')){

            /* print a blank */
            putchar(BLANK);
            /* read next char */
            c = getchar();

            /* while after the ' ' or '\t' the char is again ' ' or '\t' ... */
            /* I'm not going to bother with it and I'm going to read the next char */
            while((c == ' ') || (c == '\t')){
                c=getchar();
            }

            /* print the char */
            putchar(c);
        }

        /* another char */
        else {
            putchar(c);
        }
    }

    
}