7
\$\begingroup\$

K&R 1.9 exercise:

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

Is this correct and proper code? Could I have put c and inspace on a single line?

#include <stdio.h>

int main(void)
{
    /*initializes c and inspace. */
    int c;
    int inspace;

    /*sets inspace = 0*/
    inspace = 0;

    while((c = getchar()) != EOF)
    {
        if(c == ' ')
        {
            if(inspace ==0)
            {
                inspace = 1;
                putchar(c);
            }
        }

        if(c != ' ')
        {
            inspace = 0;
            putchar(c);
        }
    }

    return 0;
}

I am using the latest Ubuntu distro 64 bit.

\$\endgroup\$
2
  • \$\begingroup\$ What is your operating system? How do you launch your program? On unix-like operating systems, one way to do it is to type this in a terminal (without the $): $ echo "a really nice input !" | ./your_executable \$\endgroup\$ – Quentin Pradet Mar 1 '12 at 9:08
  • 1
    \$\begingroup\$ You should get rid of your comments. They do not add anything to the program. The first one is actually wrong (you're not initializing anything, you're declaring). You CAN initialize in the declaration though by using int inspace = 0;, turning two lines into one. \$\endgroup\$ – jmq Mar 1 '12 at 17:19
10
\$\begingroup\$

Looks fine to me. But I would replace

if (inspace == 0)

with

if (!inspace)

for stylistic reasons. And

if (c != ' ')

with

else 

because there is no need to compare c again: you already know it isn't equal to a space because of the first if statement.

Also the code inside {...} after main should be indented over.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ What does !inspace mean? \$\endgroup\$ – 11D Reality Hacker Feb 22 '12 at 21:56
  • \$\begingroup\$ '!' means 'not'. In a C boolean expression, 0 always means false and non-zero means true. So '!0' means true. \$\endgroup\$ – Apprentice Queue Feb 22 '12 at 22:18
  • \$\begingroup\$ In your case, if inspace really was equal to 0, then !inspace = true. I would use this style if it makes your code easier to read but it can take time to get used to. \$\endgroup\$ – Apprentice Queue Feb 22 '12 at 22:30
7
\$\begingroup\$
  • Comments should not state the obvious:

    /*initializes c and inspace. */
    int c;
    int inspace;
    
    /*sets inspace = 0*/
    inspace = 0;
    

    The first comment is also misleading because both variables are being declared (no initial value set), not initialized (initial value set). Comments are supposed to stay in sync with code, and misleading comments do detract from that.

    Furthermore, there's no need to declare and then assign to inspace. Initializing right away will also help keep variables in closer scope, which is encouraged.

    int inspace = 0;
    
  • Prefer to use isspace() to check if a character is a space. Since you're checking if a character is not a space, use it with !.

    if (!isspace(c))
    
\$\endgroup\$
0

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