0

my code problem:

this code only lets me input one command then jumps out of the loop without even inputting 'quit'


The problem is to parse a series of commands that instruct a robot arm on how to manipulate blocks that lie on a flat table. Initially, there are n blocks on the table (numbered from 0 to n − 1) with block bi adjacent to block bi+1 for all 0 ≤ i < n − 1 as shown in the diagram below:

https://ibb.co/WpWQBYT

The valid commands for the robot arm that manipulates blocks are:

• move a onto b

where a and b are block numbers, puts block a onto block b after returning any blocks that are stacked on top of blocks a and b to their initial positions.

• move a over b

puts block a onto the top of the stack containing block b, after returning any blocks that are stacked on top of block a to their initial positions.

• pile a onto b

moves the pile of blocks consisting of block a, and any blocks that are stacked above block a, onto block b. All blocks on top of block b are moved to their initial positions prior to the pile taking place. The blocks stacked above block a retain their order when moved.

• pile a over b

puts the pile of blocks consisting of block a, and any blocks that are stacked above block a, onto the top of the stack containing block b. The blocks stacked above block a retain their original order when moved.

• quit

terminates manipulations in the block world. Any command in which a = b or in which a and b are in the same stack of blocks is an illegal command. All illegal commands should be ignored and should have no effect on the configuration of blocks.

Input:

https://ibb.co/pWJ9c7Q

Output:

https://ibb.co/Nt03mm3

[I only type the code of the first command for now.]

my code:

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

int main(){
    int noi=0;
    printf("please input n:");
    int n;
    scanf(" %d",&n);
    int arr[n][n];
    int i,j;
    for(i=0;i<n;i++){
        for(j=0;j<n;j++){
            arr[i][j]=-1;
        }
        arr[i][0]=i;
    }
    char str1[5],str2[5];
    int s,d;
    
    while(strcmp(str1,"quit")!=0){
        s=0;
        d=0;
        while(!(s>=1&&s<=n&&d>=1&&d<=n)){
            scanf(" %s %d %s %d",str1,&s,str2,&d);
        }
    
        if(strcmp(str1,"move")==0){
            if(strcmp(str2,"onto"==0)){
                //empty s
                for(i=0;i<n&&arr[s][i]!=-1;i++){
                    arr[arr[s][i]][0]=arr[s][i];
                    arr[s][i]=-1;
                }
                //empty d
                for(i=0;i<n&&arr[d][i]!=-1;i++){
                    arr[arr[d][i]][0]=arr[d][i];
                    arr[d][i]=-1;
                }
                //now move s to d
                i=1;
                while(arr[d][i]!=-1){
                    i++;
                }
                arr[d][i]=arr[s][0];
                arr[s][0]=-1;
            }else if(strcmp(str2,"over")==0){
            
            }else{
                continue;
            }
        }else if(strcmp(str2,"pile")==0){
            
        }else{
            continue;
        }
    }
    
    //print results
    for(i=0;i<n;i++){
        printf("%d:",i);
        for(j=0;j<n&&arr[i][j]!=-1;j++){
            printf("%d ",arr[i][j]);
        }
        printf("\n");
    }
    
}
  • 1
    When you hit the while loop for the first time `str1` is uninitialized. This would be a good time to step through the code in a debugger to see what's going on. – Retired Ninja Dec 25 '20 at 02:02
  • 2
    `char str1[5],str2[5];` then `while(strcmp(str1,"quit")!=0){` -- On the first iteration, `str1` is **uninitialized** invoking *Undefined Behavior* when `strcmp()` attempts to access the string stored at the indeterminate address in `str1`. It could SegFault, appear to work correctly, or anything in between -- the behavior at that point is undefined. – David C. Rankin Dec 25 '20 at 02:11
  • See: [Undefined, unspecified and implementation-defined behavior](https://stackoverflow.com/q/2397984/3422102) and [What is indeterminate behavior in C++ ? How is it different from undefined behavior?](https://stackoverflow.com/q/11240484/3422102) and [Undefined behavior](https://en.cppreference.com/w/c/language/behavior) – David C. Rankin Dec 25 '20 at 02:12
  • 2
    While you posted your _code_ here as text [which is good], you posted _images_ of your sample input/output data. Please _edit_ your question and post this data in separate code blocks here. Some responders may wish to download your data [along with your code] and run your program on their machines. – Craig Estey Dec 25 '20 at 02:25
  • 2
    You must never use `"%s"` in scanf. For your case, try: `scanf(" %4s %d %4s %d",str1,&s,str2,&d);` – William Pursell Dec 25 '20 at 02:27

1 Answers1

1

There are an extremely large number of errors in your code. Probably the one of the largest impediments to recognizing the problems in your code, while not an error, is the lack of spacing in your code. Scrunching everything together makes your code very difficult to read (especially for older eyes). Open the spacing on your code up a bit.

As mentioned in the comments above, your first show-stopping problem is the use of str1 while it is uninitialized. That invokes undefined behavior and then defined operation of your code is over at that point. Your code could SegFault, appear to work normally, or anything in between.

When you are taking user-input, it is recommended you use a line-oriented input function like fgets() or POSIX getline(). That one change avoids a large number of pitfalls associated with attempting to take user-input with a formatted-input function like scanf(). If you don't know what each of the pitfalls are associated with its use -- don't use it for user input. 9 out of 10 of the user input questions on this site relate to the misuse of scanf().

Additionally, Don't Skimp On Buffer Size!!. What happens if the user enters "iskabibble" instead of "quit"? How are the characters that do not fit in str1[5] handled? What if the cat steps on the keyboard and 100 characters are entered? It's better to be 10,000 characters too long, than one character too short. Take input with fgets() and a sufficiently sized buffer, and then parse the values needed with sscanf() instead of trying to do both with scanf(), e.g.

#define MAXC 512            /* if you need a constant, #define one (or more)
                             *     ( don't skimp on buffer size!! )
                             */

int main (void) {
    
    char buf[MAXC];             /* buffer to store all user input */
    int n;                      /* if possible, declare variables at beginning of scope */
    
    fputs ("please input n for (n x n array): ", stdout);
    if (!fgets (buf, MAXC, stdin)) {                /* read all user input with fgets() */
        puts ("(user canceled input)");
        return 0;
    }
    if (sscanf (buf, "%d", &n) != 1) {              /* validate every conversion */
        fputs ("error: invalid integer input.\n", stderr);
        return 1;
    }

Reusing a single buffer to handle user-input simplifies things greatly.

While it is fine to use a VLA (Variable Length Array) for practice problems, be aware compilers are not required to support them from C11 forward, and none were supported before C99. Better to dynamically allocate for production code.

Your variable declarations should be in the scope where the variables are needed. This helps prevent variable shadowing of common variables such as i, j, etc.. 300 lines down in your code. For example, loop variables can be declared as part of the loop declaration so long as they are not needed outside the loop, e.g.

    int arr[n][n];              /* VLA's are an 'optional' feature since C11 */
    
    for (int i = 0; i < n; i++) {               /* i, j can be decalred with loop scope */
        for (int j = 0; j < n; j++) {
            arr[i][j] = -1;
        }
        arr[i][0] = i;
    }

When you need to user to enter specific input, better to loop continually until the user provides valid input (respecting their ability to cancel input by generating a manual EOF with Ctrl + d, or Ctrl + z on windows). For example where str1, s, str2 and d are needed:

    while (strcmp (buf, "quit")) {              /* loop until quit */
        char str1[MAXC] = "", str2[MAXC] = "";
        int s = 0, d = 0;
        
        while (1) {     /* loop continually */
            fputs ("enter str1 s str2 d: ", stdout);    /* prompt */
            if (!fgets (buf, MAXC, stdin)) {            /* read / validate input */
                puts ("(user canceled input)");
                return 0;
            }
            buf[strcspn (buf, "\n")] = 0;               /* trim '\n' from end of buf */
            
            if (strcmp (buf, "quit") == 0)              /* if "quit", break */
                break;
            /* parse and validate separate values, always protect array bounds */
            if (sscanf (buf, "%511s %d %511s %d", str1, &s, str2, &d) != 4) {
                fputs ("  error: invalid format or integer input.\n", stderr);
                continue;
            }   /* validate range of integers (negated conditions are confusing) */
            if ((0 <= s && s < n) && (0 <= d && d < n))
                break;  /* exit loop on good input */
            else        /* otherwise, handle error */
                fputs ("  error: value for s or d out of range.\n", stderr);
        }

(note: fgets() reads and includes the '\n' generated by the user pressing Enter, so before comparing for "quit" you will need to remove the newline with strcspn())

Since str1 and str2 are parsed from buf using sscanf() there is no '\n' to remove. However, when using sscanf() you must use the field-width modifier to protect the array bounds from overrun -- otherwise the use of scanf() or sscanf() to fill the character array is no safer than gets(), see: Why gets() is so dangerous it should never be used!

        if (strcmp (str1, "move") == 0) {           /* handle move */
            if (strcmp (str2, "onto") == 0) {       /* onto? */
                int i;                              /* declare i in scope needed */
                
                // empty s
                for (i = 0; i < n && arr[s][i] != -1; i++) {
                    arr[arr[s][i]][0] = arr[s][i];
                    arr[s][i] = -1;
                }
                
                // empty d
                for (i = 0; i < n && arr[d][i] != -1; i++){
                    arr[arr[d][i]][0] = arr[d][i];
                    arr[d][i] = -1;
                }
                
                // now move s to d
                i = 1;
                while (arr[d][i] != -1) {
                    i++;
                }
                
                arr[d][i] = arr[s][0];
                arr[s][0] = -1;
            }
            else if (strcmp (str2, "over") == 0) {
                (void)str2;                         /* no-op prevents empty scope */
            }
            else {
                continue;
            }
        }
        else if (strcmp (str2, "pile") == 0) {
            (void)str2;
        }
        else {
            continue;
        }

(note: the final else is not needed)

Complete Code

While I am still unclear on what your logic is supposed to do, handing the input can be done as shown above. Fixing the logic is left to you.

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

#define MAXC 512            /* if you need a constant, #define one (or more)
                             *     ( don't skimp on buffer size!! )
                             */

int main (void) {
    
    char buf[MAXC];             /* buffer to store all user input */
    int n;                      /* if possible, declare variables at beginning of scope */
    
    fputs ("please input n for (n x n array): ", stdout);
    if (!fgets (buf, MAXC, stdin)) {                /* read all user input with fgets() */
        puts ("(user canceled input)");
        return 0;
    }
    if (sscanf (buf, "%d", &n) != 1) {              /* validate every conversion */
        fputs ("error: invalid integer input.\n", stderr);
        return 1;
    }
    
    int arr[n][n];              /* VLA's are an 'optional' feature since C11 */
    
    for (int i = 0; i < n; i++) {               /* i, j can be decalred with loop scope */
        for (int j = 0; j < n; j++) {
            arr[i][j] = -1;
        }
        arr[i][0] = i;
    }
    
    while (strcmp (buf, "quit")) {              /* loop until quit */
        char str1[MAXC] = "", str2[MAXC] = "";
        int s = 0, d = 0;
        
        while (1) {     /* loop continually */
            fputs ("enter str1 s str2 d: ", stdout);    /* prompt */
            if (!fgets (buf, MAXC, stdin)) {            /* read / validate input */
                puts ("(user canceled input)");
                return 0;
            }
            buf[strcspn (buf, "\n")] = 0;               /* trim '\n' from end of buf */
            
            if (strcmp (buf, "quit") == 0)              /* if "quit", break */
                break;
            /* parse and validate separate values, always protect array bounds */
            if (sscanf (buf, "%511s %d %511s %d", str1, &s, str2, &d) != 4) {
                fputs ("  error: invalid format or integer input.\n", stderr);
                continue;
            }   /* validate range of integers (negated conditions are confusing) */
            if ((0 <= s && s < n) && (0 <= d && d < n))
                break;  /* exit loop on good input */
            else        /* otherwise, handle error */
                fputs ("  error: value for s or d out of range.\n", stderr);
        }
    
        if (strcmp (str1, "move") == 0) {           /* handle move */
            if (strcmp (str2, "onto") == 0) {       /* onto? */
                int i;                              /* declare i in scope needed */
                
                // empty s
                for (i = 0; i < n && arr[s][i] != -1; i++) {
                    arr[arr[s][i]][0] = arr[s][i];
                    arr[s][i] = -1;
                }
                
                // empty d
                for (i = 0; i < n && arr[d][i] != -1; i++){
                    arr[arr[d][i]][0] = arr[d][i];
                    arr[d][i] = -1;
                }
                
                // now move s to d
                i = 1;
                while (arr[d][i] != -1) {
                    i++;
                }
                
                arr[d][i] = arr[s][0];
                arr[s][0] = -1;
            }
            else if (strcmp (str2, "over") == 0) {
                (void)str2;                         /* no-op prevents empty scope */
            }
            else {
                continue;
            }
        }
        else if (strcmp (str2, "pile") == 0) {
            (void)str2;
        }
        else {
            continue;
        }
    }
    
    // print results
    for (int i = 0; i < n; i++) {
        printf ("%d:\n", i);
        for (int j = 0; j < n; j++) {
            if (arr[i][j] != -1)
                printf (" % 3d", arr[i][j]);
            else
                fputs ("  [ ]", stdout);
        }
        putchar ('\n');
    }
}

Example Use/Output

With intentional errors in input:

$ ./bin/vla_quit
please input n for (n x n array): 5
enter str1 s str2 d: move 2 onto 3
enter str1 s str2 d: move bananas onto gorillas
  error: invalid format or integer input.
enter str1 s str2 d: move 1 onto 4
enter str1 s str2 d: move -1 onto 2
  error: value for s or d out of range.
enter str1 s str2 d: move 0 onto 2
enter str1 s str2 d: quit
0:
  [ ]  [ ]  [ ]  [ ]  [ ]
1:
  [ ]  [ ]  [ ]  [ ]  [ ]
2:
  [ ]  [ ]  [ ]  [ ]  [ ]
3:
  [ ]  [ ]  [ ]  [ ]  [ ]
4:
  [ ]  [ ]  [ ]  [ ]  [ ]

Always compile with warnings-enabled, and do not accept code until it compiles without warning. To enable warnings add -Wall -Wextra -pedantic to your gcc/clang compile string (also consider adding -Wshadow to warn on shadowed variables). For VS (cl.exe on windows), use /W3. All other compilers will have similar options. Read and understand each warning -- then go fix it. They will identify any problems, and the exact line on which they occur. You can learn a lot by listening to what your compiler is telling you.

Let me know if you have further questions.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
  • I still don't know what's wrong with my scanf. My professor said scanf is good enough – Mikson Ford Dec 26 '20 at 01:49
  • @MiksonFord - I apologize, I thought I explained that, but I guess that got lost in the code.`str1` and `str2` being uninitialized cases undefined behavior. Then what does `!(s>=1&&s<=n&&d>=1&&d<=n)` do? Aside fr/om accepting the input when `0 < s < n+1` or `0 < d < n+1`? It allows input of `n` for `s` and `d` which is *one past the end* of your `arr` -- also invoking undefined behavior. Above all, since you do not ***check the return*** of any of the `scanf()` calls, any *matching failure* results in an endless loop. Not including a *field width* on `str1`, `str2` can also result in UB. – David C. Rankin Dec 26 '20 at 10:23
  • Why "move 3 onto 4" would cause matching failure when n is 7 – Mikson Ford Dec 27 '20 at 01:55
  • Those were simply examples that I dreamed up out of thin-air. The only rhyme or reason to them was to keep both within the size of `n` and `s < d`. – David C. Rankin Dec 27 '20 at 06:08