3
\$\begingroup\$

Title is pretty self explanatory, this program just takes in input (not from command line arguments), removes the extra spaces in the input, then prints that to output.

/*replace string of blanks with single blank*/

#include "stdio.h"

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

    int nb,c = 0;

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

           if (c == ' ' || c == '\n' || c == '\t')

                 if (nb >= 1)

                    nb = nb + 1;

                 else
                    nb = 1;
           else

                nb = 0;
        if (nb <= 1)
            putchar(c);

    }
    return 0;
}

Any suggestions on how to improve it?

\$\endgroup\$

4 Answers 4

4
\$\begingroup\$

A few suggestions:

  • Extract the space removal to a function of its own. It a repeatable task and despite this being a short program it's still good practice. Then all you have to do in main() is just pass in the input string. Here's a function I came up with:

    void removeSpaces(char* source)
    {
        for (char *i = source, *j = source; *j != 0;)
        {
            *i = *j++;
            if(*i != ' ') i++;
        }
        *i = 0;
    }
    

    Keep in mind that this method modifies the original string.

  • Change how you get input from the user. I'd recommend either getting input via command line arguments, or using fgets(). It's safer those ways and easier to do in my opinion.

  • If you decide to not use command line arguments, declare your function as:

    int main(void)
    
  • Put the variable declarations to separate lines and initialize them to some value. From Code Complete, 2d Edition, p. 759:

    With statements on their own lines, the code reads from top to bottom, instead of top to bottom and left to right. When you’re looking for a specific line of code, your eye should be able to follow the left margin of the code. It shouldn’t have to dip into each and every line just because a single line might contain two statements.

  • You don't have to return 0 at the end of main(), just like you wouldn't bother putting return; at the end of a void-returning function. The C standard knows how frequently this is used, and lets you not bother.

    C99 & C11 §5.1.2.2(3)

    ...reaching the } that terminates the main() function returns a value of 0.

\$\endgroup\$
4
\$\begingroup\$

syb0rg suggested already to initialize all variables, and actually the uninitialized variable nb causes a potential error in your program: An initial space character might not be preserved.

If you initialize the variable to zero

int nb = 0;

the check if (nb >= 1) after finding a space character is not necessary anymore, and that part can be simplified to

if (c == ' ' || c == '\n' || c == '\t')
    nb = nb + 1;
else
    nb = 0;

The output of the program still contains all kind of whitespace (space, newline or tab character), because the first whitespace character in a sequence is preserved. This means that

a<space><NL>b   is transformed to   a<space>b
a<NL><space>b   is transformed to   a<NL>b

It might be a good idea to replace all whitespace sequences by a space character:

if (c == ' ' || c == '\n' || c == '\t') {
    if (nb == 0) {
        putchar(' ');
    }
    nb = nb + 1;
} else {
    nb = 0;
    putchar(c);
}

You can also use isspace() to check for whitespace characters (but note that this function is locale-dependent).

\$\endgroup\$
3
\$\begingroup\$
  • The #include should be <stdio.h>, not "stdio.h". The quotes are only for user headers, which this is not.

  • I have no idea what these mean:

    int nb,c = 0;
    

    Don't use single-character variables unless they're either very obvious or are for loop counter variables (which would be in the loop statements anyway). Use actual variable names so that the reader won't be left guessing.

  • Your curly brace usage is inconsistent. If a conditional statement body is on the next line, surround it with curly braces, even if it's only one line. There should especially be curly braces around the larger if statement.

\$\endgroup\$
3
\$\begingroup\$

My main question when reading your code is: Why do you count the number of spaces? You don't need the number of spaces, only to know whether the previous character was whitespace and if the current character is whitespace.

In addition there are some style issues in your code:

  • Keep the braces even for one-line blocks – In your code you leave out the braces around both of your if loops, and it does work, but you are digging a grave for your self leaving out the braces.

    You also have a visual indentation error when indenting the first if block. The combination of indentation errors and not using braces will at some point in time introduce bugs and errors. So it is a good thing to get used to always using the braces with if, while and for loops.

  • Use better names – You can use very short variable names, but it is better to use descriptive names like numberOfBlanks instead of nb. Or currentChar instead of c. An exception is the use if i, j, ... in loops, and for some to use c and str for a character or a text string. So choose your names wisely to make your code as readable as possible.

Refactored code

Applying these two elements, and changing to use include <stdio.h> we get the following code:

#include <stdio.h>

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

    int previousWasWhitespace = 0;
    int currentIsWhitespace = 0;
    int c = 0;

    while ((c = getchar()) != EOF) {
        currentIsWhitespace = c == ' ' || c == '\n' || c == '\t';  

        if  (!(previousWasWhitespace && currentIsWhitespace)) {
            putchar(c);
        }

        previousWasWhitespace = currentIsWhitespace;
    }
}

Notice how you now can read the if statement directly as: if not previousWasWhitespace and currentIsWhiteSpace putchar. And this should be a goal, that the code should as readable as possible.

On a side note, the if statement can be reversed into if ( !previousWasWhiteSpace || !currentIsWhitespace ), but whilst still correct it is harder to read, so I prefer the first variation just because of readability.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.