CODEWITHSUNDEEP
c
TheAJ
TheAJ

Reputation: 10875

Scanf keeps looping

Well i have this code to find the paint quality of a room.

void get_room_size(char room_id, int * length, int * width) {
    while (*length <= 0 && *width <= 0) {
        printf("Enter length and width of room %c in feet: ", room_id);
        if (scanf("%d,%d", length, width)) {
            if (*length <= 0) {
                printf("###Error! Length must be a positive value!\n");
            }
            if (*width <= 0) {
                printf("###Error! Width must be a positive value!\n");
            }
            printf("\n");
        } else {
            printf("bad data");
            *length = 0;
            *width = 0;
        }
    }
}

Basically, if i enter

a,1

It will go crazy and keep looping. Whats the problem?

Upvotes: 1

Views: 2174

Answers (4)

paulsm4
paulsm4

Reputation: 121609

Try this:

...
       if (scanf("%d,%d", length, width) == 2) {
            if (*length <= 0) {
                printf("###Error! Length must be a positive value!\n");
            }
            if (*width <= 0) {
                printf("###Error! Width must be a positive value!\n");
            }
            printf("\n");
        } else {
            printf("bad data");
            *length = -1;
            *width = -1;
        }

Upvotes: 0

sam_k
sam_k

Reputation: 6023

You dont put & in scanf() statement so how its read

if (scanf("%d,%d", &length, &width))

Upvotes: -1

paxdiablo
paxdiablo

Reputation: 881113

The reason it's going "crazy" is as follows. When scanf fails to read in a as a number (because it's not numeric, obviously), it won't advance the file pointer.

This is one reason why you shouldn't generally use scanf operations, a failure can leave the file pointer in an indeterminate position (such as if you only scan in 3 of 12 items).

The other reason is that scanf means "scan formatted" and you would be hard pressed finding anything more unformatted than user input.

Anyway, back to the failure. Because the file pointer isn't advanced, the next time you come back to do fscanf, it will try to read that a again (and again and again).

If you want a decent function for handling user input, look no further than here:

#include <stdio.h>
#include <string.h>

#define OK       0
#define NO_INPUT 1
#define TOO_LONG 2
static int getLine (char *prmpt, char *buff, size_t sz) {
    int ch, extra;

    // Get line with buffer overrun protection.
    if (prmpt != NULL) {
        printf ("%s", prmpt);
        fflush (stdout);
    }
    if (fgets (buff, sz, stdin) == NULL)
        return NO_INPUT;

    // If it was too long, there'll be no newline. In that case, we flush
    // to end of line so that excess doesn't affect the next call.
    if (buff[strlen(buff)-1] != '\n') {
        extra = 0;
        while (((ch = getchar()) != '\n') && (ch != EOF))
            extra = 1;
        return (extra == 1) ? TOO_LONG : OK;
    }

    // Otherwise remove newline and give string back to caller.
    buff[strlen(buff)-1] = '\0';
    return OK;
}

This will input a line from the user, with overflow protection (unlike gets or scanf with unbounded "%s").

It also flushes to end of line if the input was too long, which will stop the remainder of the line from affecting the next input operation.

You can then sscanf the buffer to your heart's content without any concerns re the file pointer.

The following test program shows how to use this:

int main (void) {
    int rc;
    char buff[10];

    rc = getLine ("Enter string> ", buff, sizeof(buff));
    if (rc == NO_INPUT) {
        // Extra NL since my system doesn't output that on EOF.
        printf ("\nNo input\n");
        return 1;
    }

    if (rc == TOO_LONG) {
        printf ("Input too long [%s]\n", buff);
        return 1;
    }

    printf ("OK [%s]\n", buff);

    return 0;
}

As an aside, you may want to re-examine your logic for a valid sized room. What you currently have would allow a room to be entered as 7 by -42 feet :-)

It's also not usually good form to rely on the output values being set to specific values on entry. If length and width are (for example) 3 and 4 on entry, this function will exit straight away without asking the user for input.

The first problem can be fixed by using || instead of &&. The second by initialising the variables to 0 at the start of the function so that the loop is entered.

For completeness, if you combine that original snippet above (the include statements and the getLine() function) with the following slightly modified get_room_size() function:

static void get_room_size(char room_id, int * length, int * width) {
    char buff[100];
    *length = *width = 0;
    while ((*length <= 0) || (*width <= 0)) {
        printf("Enter length and width of room %c in feet: ", room_id);
        int rc = getLine (NULL, buff, sizeof (buff));

        if (rc == NO_INPUT) {
            printf ("\nEnd of file encountered.\n");
            return;
        }

        if (rc == TOO_LONG) {
            printf ("\nInput too long, please retry.\n");
            continue;
        }

        if (sscanf(buff, "%d,%d", length, width) != 2) {
            *length = *width = 0;
            printf ("\nInput not in desired form (<number>,<number>), "
                "please retry.\n");
            continue;
        }

        if ((*length <=0) || (*width <= 0)) {
            *length = *width = 0;
            printf ("\nBoth length and width must be greater than zero, "
                "please retry.\n");
        }
    }
}

and the very simple test main(), you'll see a complete program showing how to do it.

int main (void) {
    int len, wid;
    get_room_size ('x', &len, &wid);
    printf ("Length is %d, width is %d.\n", len, wid);
    return 0;
}

Upvotes: 6

Alex Force
Alex Force

Reputation: 9

your scanf is taking in two integer. A is a character, think you want to scanf("%c", *room_id); scanf("%d", length);

I reccommend you do them separately

Upvotes: 0

Related Questions