0

I'm working on this problem(Uva227).I've almost received correct results when I test the cases, but I've met a problem. I used function gets() to get a line from standard input, and store it in a[0]. but after the first input matrix, a[0] never changes anymore. What's wrong with it?

P.S. In Puzzle #2(i.e., the second input matrix), a[0] didn't changed, so I got a wrong answer. But if I put it first, it returned a correct output. So I think the algorithm is correct, but something went wrong when reading the first line.

English is not my native language; please excuse my syntax errors.

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

char a[5][5];



int main(){
    int _case = 1;
    char c;
    while(gets(a[0])){//Please focus on this line;it seems that a[0] never changed since first reading.
        if(a[0][0] == 'Z')  break;
        int blank_i, blank_j;
        for(int i = 0; i < 5; i++){
            if(i)   gets(a[i]);
            for(int j = 0; j < 5; j++){
                if(a[i][j] == ' '){
                     blank_i = i;
                     blank_j = j;
                }
            }
        }
        bool flag = true;
        int i = 0;
        while((c = getchar()) != '0'){
            switch(c){
                case 'A':
                    a[blank_i][blank_j] = a[blank_i - 1][blank_j];
                    a[--blank_i][blank_j] = ' ';    break;
                case 'B':
                    a[blank_i][blank_j] = a[blank_i + 1][blank_j];
                    a[++blank_i][blank_j] = ' ';    break;
                case 'L':
                    a[blank_i][blank_j] = a[blank_i][blank_j - 1];
                    a[blank_i][--blank_j] = ' ';    break;
                case 'R':
                    a[blank_i][blank_j] = a[blank_i][blank_j + 1];
                    a[blank_i][++blank_j] = ' ';    break;
                default:    break;
             }
        }
        //add a getchar() here will fix the problem.
        printf("Puzzle #%d:\n", _case);
        if(blank_i < 0 || blank_i > 4 || blank_j < 0 || blank_j >  4)
             printf("This puzzle has no final configuration.\n");
        else{
            for(int i = 0; i < 5; i++){
                for(int j = 0; j < 5; j++){
                    printf("%c ", a[i][j]);
                }
            printf("\n");
            }
        }
        printf("\n");
        _case++;
    }
    return 0;
}
Leon.L
  • 13
  • 3
  • 2
    Never use gets. It's a dangerous function. Use fgets instead. – klutt Jun 02 '19 at 02:52
  • 1
    [Why gets() is so dangerous it should never be used!](https://stackoverflow.com/questions/1694036/why-is-the-gets-function-dangerous-why-should-it-not-be-used) Use `if (fgets (a[i], 5, stdin)) a[i][strcspn(a[i], "\n")] = 0; else { /* handle error */ }` – David C. Rankin Jun 02 '19 at 02:58

2 Answers2

1

You've misdiagnosed the problem. It's not the case that "after the first input matrix, a[0] never changes anymore." The actual problem is that after you read the '0' that ends the list of moves, there's a newline before the next puzzle begins. Your code doesn't take this into account, and so treats the newline as the first character of the first line of the next puzzle. Fix it by swallowing this newline before getting back to the beginning of the while loop.

Aside: Some commenters pointed out that you shouldn't use gets. They're 100% right, but it's not related to this problem, and switching away from it won't fix this.

  • *"switching away from it won't fix this."* - true, but it *will* bring compliance to the C code itself, since `gets` was completely removed from the standard library starting in C11. I'm not surprised, but rather disappointed that UVA isn't using C11 compliance, in which case the code wouldn't just deliver a wrong answer; it wouldn't even *compile*. Regardless, have an uptick. – WhozCraig Jun 02 '19 at 03:19
  • I added a getchar() before output the matrix and it worked. Thank you very much! – Leon.L Jun 02 '19 at 03:19
0

In additions to Joeseph's answer, you invite Undefined Behavior by failing to validate that your i, j, blank_i, blank_j indexes remain in bounds. (and *Never, ever use gets() it has been removed from the current C standard). See: Why gets() is so dangerous it should never be used!

Instead, use fgets and trim the trailing '\n' from the resulting string by overwriting with the nul-characer, e.g.

#define ROWS 5      /* if you need a constant, #define one (or more) */
#define COLS ROWS

char a[ROWS][COLS];
...
        for(int i = 0; i < ROWS; i++){
            if (i)                                  /* skips a[0] */
                if (fgets (a[i], COLS, stdin))
                    a[i][strcspn(a[i], "\n")] = 0;  /* overwrite '\n' */
                else {  /* handle failed input */
                    fputs ("error: EOF encountered.\n", stderr);
                    return 1;
                }
           ...

Next, in the following for loop you assign blank_i = i; and blank_j = j;, e.g.

            for (int j = 0; j < COLS; j++){
                if(a[i][j] == ' ') {
                     blank_i = i;
                     blank_j = j;
                }
            }

Subsequently, you then increment a[blank_i++][blank_j]; and used blank_j-- and blank_j++ (you must also ensure blank_i-- never results in a negative value). To avoid Undefined Behavior, you must validate your indexes remain in range. For example, taking your case 'B':, you can do something similar to:

                case 'B':
                    {   /* create new block to allow variable declaration
                         * blank_i + 1 can exceed 4 invokding Undefined
                         * Behavior. You must validate/handle value.
                         */
                        int row = blank_i + 1 % ROWS;
                        a[blank_i][blank_j] = a[row][blank_j];
                        /* same with ++blank_i */
                        blank_i = (blank_i + 1) % ROWS;
                        a[blank_i][blank_j] = ' ';
                        break;  /* lines don't cost $, done hide break at end */
                    }

Since you are looping while ((c = getchar()) != '0') both values blank_i and blank_j can exceed your array bounds or result in a negative index invoking Undefined Behavior -- long before your ever check:

        printf("Puzzle #%d:\n", _case);
        if(blank_i < 0 || blank_i > 4 || blank_j < 0 || blank_j >  4)
             printf("This puzzle has no final configuration.\n");

Go through and fix your indexing (handling it any way you need) to validate your indexes remain within the bounds of your array.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85