0

I've been learning C on my own and have been re-working a program out of the C Primer book. I was hoping a fresh set of eyes could possibly spot the one issue that I'm having. As you can see by my output vs expected output I would like to get rid of the line "0 is a number". I believe a re-tooling of the while loop is the issue, but I can't seem to get rid of it despite the variations I've tried.

Output:

    Enter some integers. Enter 0 to end.
    1 two 3 0 4
    1 is a number.
    two is not an integer
    3 is a number.
    0 is a number.

Expected output:

    Enter some integers. Enter 0 to end.
    1 two 3 0 4
    1 is a number.
    two is not an integer
    3 is a number.

#include <stdio.h>
#include <ctype.h>

int get_int(void); //validate that input is an integer

int main(void)
{
    int integers;

    printf("Enter some integers. Enter 0 to end.\n");
    while (integers != 0)
    {
        integers = get_int();
        printf("%d is a number\n", integers);
    }
    return(0);

} // end main

int get_int(void)
{
    int input;
    char ch;

    while (scanf("%d", &input) != 1)
    {
        while (!isspace(ch = getchar()) )
            putchar(ch); //dispose of bad input
        printf(" is not an integer\n"); 
    }    
    return input;
 }// end get_int
Improve this question
1
  • 3
    Note that your loop as currently written is not guaranteed to execute at all. It is possible that integers would contain 0 before the loop is executed. Using uninitialized variables leads to bugs. If you compile with optimization and warnings, GCC will report that (gcc -O3 -Wall should do it; I use -Wextra too routinely). Incidentally, IIRC, on Solaris, the stack is mostly zeroed, so there'd be a fairly good chance that integers is zero on entry to the program. Commented May 22, 2013 at 16:02

4 Answers 4

Reset to default
6

What I would do is move the call to get_int into the condition of the while loop:

int main(void)
{
    int integers;

    printf("Enter some integers. Enter 0 to end.\n");
    while ((integers = get_int()) != 0)
    {
        printf("%d is a number\n", integers);
    }
    return(0);

} // end main

The problem with your existing code is that between calling get_int() and printing the value, you're not checking to see if it's returned your sentinel of 0.

The other option would be to add an if (integers == 0) { break; } condition in between, but in my mind doing the assignment in the condition is cleaner.

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

I've been known to write this loop this way myself, but if the initial processing and condition are any more complicated than this, I think the internal "break" statement is clearer. One case that comes up a lot is for (;;) { c = getchar(); if (c == EOF || c == terminator) break; ... } -- you can write that entirely inside a while condition but only by assigning-and-testing c on the left side of a && and testing it again on the right, which is like to confuse people.
2

You are correct to suspect that you need to retool the while loop. Did you try something like this?

 for (;;)
 {
     integers = get_int();
     if (integers == 0) break;
     printf("%d is a number\n", integers);
 }

Also, your get_int would be better written with fgets (or getline if available) and strtol. scanf is seductively convenient but almost always more trouble than it's worth.

Improve this answer

1 Comment

This is one of Knuth's 'loop-and-a-half' constructs, which you can find more about with a Google search on 'Knuth loop-and-a-half'.
2

The simplest way is to put your condition and assignment into the while-loop. Right now your code relies on integer being set in the loop, then loops again to check if it's zero. 

while((integer = get_int()) != 0)

will let you check at the same time as assigning integer. Don't forget the parenthesis, or your integer value will be the result of integer = (get_int != 0), because != has higher priority than = in C and C++.

Improve this answer

Comments

1

Consider the core of your loop:

integers = get_int();
printf("%d is a number\n", integers);

No matter what get_int() returns, the printf line will be executed. That line needs a separate if:

integers = get_int();
if (integers != 0) printf("%d is a number\n", integers);
Improve this answer

4 Comments

But repeating the loop condition in the body of the loop is ugly. What you've done works, but it is ugly!
@JonathanLeffler I had quite a think before I posted the answer. There are various ways of solving this problem: using break (as per Zack) - adds a line, adds a conditional, uses break (so harder to follow flow imho); add a conditional (as per me) ugly as you say from a code purist point of view but easy for someone learning the language to learn from or John Ledbetter's solution (ideal from the code purist point of view but puts a lot of functionality into one line which can be harder for a learner to grasp from). Caveat Emptor!
www-cs-faculty.stanford.edu/~eroberts//papers/SIGCSE-1995/… makes a really compelling argument IMNSHO for the break-in-the-middle being easier to understand than any of the alternatives.
@Zack guess it's just me that find's break hard to follow - it always leaves me with a feeling of insecurity - break to where? But I guess that's just me :-) The good thing is to see all three answers up here - as an overall learning experience for the OP, it's ideal, I would think.

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.