Thread: Why isn't my code working the way I need?

  1. #1
    Registered User
    Join Date
    Jun 2016
    Posts
    14

    Why isn't my code working the way I need?

    The code below is supposed to fill a 10 x 10 character array with the element '.' and then walk through the matrix randomly using the rand() function printing the alphabet along with the movement. There's nothing random about what's priniting for me. In fact, this is all I get no matter how many times I compile and run:

    D A . . . . . . . .
    C B . . . . . . . .
    . . . . . . . . . .
    . . . . . . . . . .
    . . . . . . . . . .
    . . . . . . . . . .
    . . . . . . . . . .
    . . . . . . . . . .
    . . . . . . . . . .
    . . . . . . . . . .




    Code:
    #include <stdio.h>
    #include <stdlib.h>
    
    int main()
    {
        // main declarations
        char matrix[10][10], *cursor, *start;
        int direction;
        unsigned char elem = 'A';
        
        
        
        
        // fill the matrix elements with .
        for (cursor = &matrix[0][0]; cursor <= &matrix[9][9]; cursor++)
        {
            *cursor = '.';
        }
        
        
        
        
        // walk randomly through the matrix, printing the alphabet in order as we go
        start = cursor = &matrix[0][0];
        while (elem < 91)
        {
            direction = rand() % 4;
            switch (direction)
            {
                Up:
                case 0:
                    if ((*(cursor+1) != '.') && (*(cursor-1) != '.') && (*(cursor+10) != '.') && (*(cursor-10) != '.'))
                        goto printMatrix;
                    
                    if (cursor > (start + 9))
                        cursor -= 10;
                    else goto Right;
                        
                    if (*cursor == '.')
                        *cursor = elem;
                    else
                    {
                        cursor += 10;
                        goto Right;    
                    }
                    
                    break;
                    
                
                Right:    
                case 1:
                    if ((*(cursor+1) != '.') && (*(cursor-1) != '.') && (*(cursor+10) != '.') && (*(cursor-10) != '.'))
                        goto printMatrix;
                    
                    if ((cursor != &matrix[0][9]) || (cursor != &matrix[1][9]) || (cursor != &matrix[2][9]) || (cursor != &matrix[3][9]) || (cursor != &matrix[4][9]) || (cursor != &matrix[5][9]) || (cursor != &matrix[6][9]) || (cursor != &matrix[7][9]) || (cursor != &matrix[8][9]) || (cursor != &matrix[9][9]))
                        cursor++;
                    else goto Left;
                    
                    if (*cursor == '.')
                        *cursor = elem;
                    else
                    {
                        cursor--;
                        goto Left;    
                    } 
                    
                    break;
                    
                
                Left:    
                case 2:
                    if ((*(cursor+1) != '.') && (*(cursor-1) != '.') && (*(cursor+10) != '.') && (*(cursor-10) != '.'))
                        goto printMatrix;
                    
                    if ((cursor != &matrix[0][0]) || (cursor != &matrix[1][0]) || (cursor != &matrix[2][0]) || (cursor != &matrix[3][0]) || (cursor != &matrix[4][0]) || (cursor != &matrix[5][0]) || (cursor != &matrix[6][0]) || (cursor != &matrix[7][0]) || (cursor != &matrix[8][0]) || (cursor != &matrix[9][0]))
                        cursor--;
                    else goto Down;
                    
                    if (*cursor == '.')
                        *cursor = elem;
                    else
                    {
                        cursor++;
                        goto Down;    
                    } 
                    
                    break;
                    
                
                Down:    
                case 3:
                    if ((*(cursor+1) != '.') && (*(cursor-1) != '.') && (*(cursor+10) != '.') && (*(cursor-10) != '.'))
                        goto printMatrix;
                    
                    if (cursor < (start + 90))
                        cursor += 10;
                    else goto Up;
                    
                    if (*cursor == '.')
                        *cursor = elem;
                    else
                    {
                        cursor -= 10;
                        goto Up;    
                    } 
                    
                    break;
            }
            elem++;
        }
        
        
        
        
        // print the matrix
        printMatrix:
            for (int i = 0, j = 0; i < 10; i++)
            {
                for (j = 0; j < 10; j++)
                {
                    printf("%c ", matrix[i][j]);
                }
                printf("\n");
            }
            
            
            
            
        // end main
        return 0;
    }

  2. #2
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    39,660
    Maybe you should try simplifying the code and removing all those goto's.

    > if ((cursor != &matrix[0][9]) || (cursor != &matrix[1][9]) || (cursor != &matrix[2][9]) || (cursor != &matrix[3][9]) || (cursor != &matrix[4][9]) || (cursor != &matrix[5][9]) || (cursor != &matrix[6][9]) || (cursor != &matrix[7][9]) || (cursor != &matrix[8][9]) || (cursor != &matrix[9][9]))
    This is a really crap way of determining if you're at an edge.
    If the matrix was 1000 by 1000, what would you do?

    Instead of having some kind of 'cursor' pointing at a specific element, and using lot's of address arithmetic, create a pair of variables called 'row' and 'column'.
    Testing for edges becomes a snap.


    > There's nothing random about what's priniting for me. In fact, this is all I get no matter how many times I compile and run:
    Well you never do something like this at the beginning of main
    srand(time(NULL));

    So your rand() always produces the same result sequence, no matter how many times you run it.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  3. #3
    Registered User
    Join Date
    Jun 2016
    Posts
    14
    Thanks, Salem. The srand(time(NULL)); line really worked, of course after I included <time.h> header.

    As for the long if statements, yes, that was stupid. I've added the rows as i and the columns as j. As for the other two long if statements (4 conditions each), I can't seem to find a better way to do them than the pointer I'm using for them. Everything is working perfectly except that it is still going off the edge to the left or right, but not up or down - i.e. the row condition seems to be working but the column edge condition doesn't seem to be. I can't figure out what the problem is.

    Also, while the cursor that is pointing to the beginning of the matrix should be filling the first element of the array when I run the statement *cursor = elem; but instead it is actually starting at the second so I had to do cursor-- before executing the Matrix Random Walk logic.

    Btw, I'm using the MinGW GCC 6.1.0 compiler on a Windows 10, 64-bit computer, with Dev-C++ as my editor.

    Below is the code:

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <time.h>
    
    int main()
    {
        // main declarations
        char matrix[10][10], *cursor;
        int direction;
        unsigned char elem = 'A';
        
        
        
        
        // fill the matrix elements with .
        for (cursor = &matrix[0][0]; cursor <= &matrix[9][9]; cursor++)
        {
            *cursor = '.';
        }
        
        
        
        
        /* walk randomly through the matrix, printing the alphabet in order as we go */
        
        
        // first some declarations and initializations for our main cursor pointer and the row and column indicators
        cursor = &matrix[0][0];
        cursor--;
        int i = 0, j = 0;
        
        
        // for rand()
        srand(time(NULL));
        
        
        // now for the main logic, the Matrix Random Walk
        while (elem < 91)
        {
            direction = rand() % 4;
            switch (direction)
            {
                Up:
                case 0:
                    if ((*(cursor+1) != '.') && (*(cursor-1) != '.') && (*(cursor+10) != '.') && (*(cursor-10) != '.'))
                        goto printMatrix;
                    
                    if (i > 0)
                    {
                        cursor -= 10;
                        i--;
                    }
                    else goto Right;
                        
                    if (*cursor == '.')
                        *cursor = elem;
                    else
                    {
                        cursor += 10;
                        i++;
                        goto Right;    
                    }
                    
                    break;
                    
                
                Right:    
                case 1:
                    if ((*(cursor+1) != '.') && (*(cursor-1) != '.') && (*(cursor+10) != '.') && (*(cursor-10) != '.'))
                        goto printMatrix;
                    
                    if (j < 9)
                    {
                        cursor++;
                        j++;
                    }
                    else goto Left;
                    
                    if (*cursor == '.')
                        *cursor = elem;
                    else
                    {
                        cursor--;
                        j--;
                        goto Left;    
                    } 
                    
                    break;
                    
                
                Left:    
                case 2:
                    if ((*(cursor+1) != '.') && (*(cursor-1) != '.') && (*(cursor+10) != '.') && (*(cursor-10) != '.'))
                        goto printMatrix;
                    
                    if (j > 0)
                    {
                        cursor--;
                        j--;
                    }
                    else goto Down;
                    
                    if (*cursor == '.')
                        *cursor = elem;
                    else
                    {
                        cursor++;
                        j++;
                        goto Down;    
                    } 
                    
                    break;
                    
                
                Down:    
                case 3:
                    if ((*(cursor+1) != '.') && (*(cursor-1) != '.') && (*(cursor+10) != '.') && (*(cursor-10) != '.'))
                        goto printMatrix;
                    
                    if (i < 9)
                    {
                        cursor += 10;
                        i++;
                    }
                    else goto Up;
                    
                    if (*cursor == '.')
                        *cursor = elem;
                    else
                    {
                        cursor -= 10;
                        i--;
                        goto Up;    
                    } 
                    
                    break;
            }
            elem++;
        }
        
        
        
        
        // print the matrix
        printMatrix:
            for (int i = 0, j = 0; i < 10; i++)
            {
                for (j = 0; j < 10; j++)
                {
                    printf("%c ", matrix[i][j]);
                }
                printf("\n");
            }
            
            
            
            
        // end main
        return 0;
    }

  4. #4
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    printMatrix really should be a function, not a label that you use goto with. Since you have a switch within a loop, you should not need to use goto to change the direction; a change of direction presumably should work merely by setting the direction variable to the desired direction value, i.e., instead of the magic numbers 0, 1, 2, and 3, you should have defined an enum with Up, Right, Left, and Down, and then ditched the unnecessary labels in favour of the case labels.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  5. #5
    Registered User
    Join Date
    Jun 2016
    Posts
    14
    Okay. It's fixed now. The changes I made to the code were only in one place - the code below:

    Code:
        // first some declarations and initializations for our main cursor pointer and the row and column indicators
        cursor = &matrix[0][0];
        *cursor = elem;
        elem++;
        int i = 0, j = 0;

    Thanks all for your help.

    P.S.: laserlight, there were three if statements in each case of the switch. If the first if statement is satisfied, then the goto printMatrix is necessary to break not only from the switch but also from the while loop since we will no longer be incrementing elem or the cursor pointer. If the second and third if statements are satisfied, then and only then is the matrix element the cursor is pointing to set to elem. Within the switch cases, the gotos and the breaks had to be there the way I coded this. Anyway, thanks much for your help and input.

  6. #6
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Quote Originally Posted by AamirYousafi
    If the first if statement is satisfied, then the goto printMatrix is necessary to break not only from the switch but also from the while loop since we will no longer be incrementing elem or the cursor pointer.
    Breaking out of nested constructs is a valid use of goto, but if so, the label should not be named printMatrix. Rather, it should be named say, endOfWalk, and then you can do whatever else you want to do, e.g., print the matrix.

    Quote Originally Posted by AamirYousafi
    If the second and third if statements are satisfied, then and only then is the matrix element the cursor is pointing to set to elem. Within the switch cases, the gotos and the breaks had to be there the way I coded this.
    Nope. From what I see, it really is spaghetti code, and I don't think it is of the Duff's device variety. You should be able to restructure it if you really took the time to think about what the code does. For example, perhaps you could introduce a loop based on the current condition for which you have your repeated goto printMatrix. Perhaps you could make use of fall through of the switch (though then you'll need a goto for wraparound), or perhaps it would be clearer to loop another time with the new direction (no, not the direction from the rand() call, but what you currently have as goto). Maybe separating each direction into a function might help.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  7. #7
    Banned
    Join Date
    Oct 2014
    Location
    Home
    Posts
    135
    Quote Originally Posted by laserlight View Post
    Breaking out of nested constructs is a valid use of goto, but if so, the label should not be named printMatrix. Rather, it should be named say, endOfWalk, and then you can do whatever else you want to do, e.g., print the matrix.


    Nope. From what I see, it really is spaghetti code, and I don't think it is of the Duff's device variety. You should be able to restructure it if you really took the time to think about what the code does. For example, perhaps you could introduce a loop based on the current condition for which you have your repeated goto printMatrix. Perhaps you could make use of fall through of the switch (though then you'll need a goto for wraparound), or perhaps it would be clearer to loop another time with the new direction (no, not the direction from the rand() call, but what you currently have as goto). Maybe separating each direction into a function might help.
    I see one thing though about standards. It is not always necessary. However you obviously probably end up with your own niche. That's why you have someone who does C instead of C++. Another example is the old timers use FORTRAN70 while when I was in school they started FORTRAN90. Believe it or not just recently they had old mainframes in the newspaper advertised for a programming position. Well I fib. I know a friend of mine who works in the power department and it is too important to just change what has worked to something new. I got the chance to see a person working on a telephone line and they are old thick clunky things. You would think they would have upgraded those by now. However the basics remains.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Why is this code not working?
    By Owais in forum C Programming
    Replies: 5
    Last Post: 04-06-2014, 06:09 AM
  2. Why the following code not working?
    By Sonam Agarwal in forum C Programming
    Replies: 6
    Last Post: 01-22-2014, 04:21 AM
  3. Can someone tell me why this code isn't working please?
    By psynt555 in forum C Programming
    Replies: 5
    Last Post: 04-24-2012, 05:14 PM
  4. why is my code not working from here it looks ok
    By Lisa_townsend in forum C Programming
    Replies: 19
    Last Post: 12-05-2009, 12:08 PM
  5. Code Not working
    By srivatsan in forum C Programming
    Replies: 4
    Last Post: 10-13-2008, 04:38 AM

Tags for this Thread