3
\$\begingroup\$

I am quite new to c, and I'm trying to get better in it. I want to create a simple snake & ladder game.

I achieved the layout of the board that I wanted, but I'm not sure if this is really an efficient way of creating it.

Could you give me some input or review to that?

#include <stdio.h>
#include <stdint.h>
#include <stdbool.h>

void initBoard(void) {
    uint8_t board[100];
    bool row_even = true;

    for (uint8_t i = 1; i < 101; i++) {
        board[i] = i;
    }
    
    for (uint8_t i = 100; i > 0; i -= 10){
        if (row_even) {                 // Check if even or odd row
            for (uint8_t k = 0; k < 11; k++) {
                if (((k % 10) == 0) && (k != 0)) {
                    printf("\n");
                    row_even = false;
                    continue;
                }
                printf("%d ", board[i - k]);
            }
        } else {
            for (int8_t k = 9; k > -1; k--) {
                if (((k % 10) == 0) && (k != 10)) {
                    printf("%d ", board[i]);
                    printf("\n");
                    row_even = true;
                    continue;
                } 
                printf("%d ", board[i - k]);
            }
        }
    }
}

int main(void) {

    initBoard();


    return 0;
}

The output is as I wanted it to be:

100 99 98 97 96 95 94 93 92 91 
81 82 83 84 85 86 87 88 89 90
80 79 78 77 76 75 74 73 72 71
61 62 63 64 65 66 67 68 69 70
60 59 58 57 56 55 54 53 52 51
41 42 43 44 45 46 47 48 49 50
40 39 38 37 36 35 34 33 32 31
21 22 23 24 25 26 27 28 29 30
20 19 18 17 16 15 14 13 12 11
01 02 03 04 05 06 07 08 09 10
\$\endgroup\$
7
  • \$\begingroup\$ Recommend using 99-0 instead of 100-1... It simply makes more sense (to me). \$\endgroup\$
    – Fe2O3
    Commented Sep 4 at 23:16
  • 2
    \$\begingroup\$ @Fe2O3: The game board usually has squares labeled 1-100. See the link in the question. \$\endgroup\$
    – toolic
    Commented Sep 5 at 0:24
  • \$\begingroup\$ @toolic That something has been used in one way in the past does not mean it is superior to a suggested alternative. See "The Tau Manifesto"... The advantage of SI units is that they are more sensible than their equivalent Imperial counterparts. The game, Snakes & Ladders, could be played equally well on a 2D spiral as a representation of "ascending" a 3D cone. Gotta think outside the box! \$\endgroup\$
    – Fe2O3
    Commented Sep 5 at 1:30
  • 1
    \$\begingroup\$ @Cats "I achieved the layout of the board that I wanted" --> Really? Where in code does the '0' come from to print 1 as "01" instead of "1" as listed in the "output is as I wanted". \$\endgroup\$
    – chux
    Commented Sep 5 at 13:32
  • 1
    \$\begingroup\$ @Fe2O3 Interesting about SI, yet computers are best used to use internally what makes coding sense yet provide the human interface per local custom as in Apollo \$\endgroup\$
    – chux
    Commented Sep 5 at 13:36

5 Answers 5

5
\$\begingroup\$

Simplify

In the "even" clause, you can simplify the check:

            if (((k % 10) == 0) && (k != 0)) {

as:

            if (k == 10) {

Similarly, in the "odd" clause, the check:

            if (((k % 10) == 0) && (k != 10)) {

can be simplified to:

            if (k == 0) {

There is no need to compare k to 10 since it is always less than 10.

Output alignment

When I run your code, I get a different output in the bottom row. The single digits are not left-padded with "0". Perhaps there is a portability issue if your compiler does the padding.

If I replace all your printf "%d " with "%3d", this sets the field width to 3 characters (right-justified). Here is the output I get:

100 99 98 97 96 95 94 93 92 91
 81 82 83 84 85 86 87 88 89 90
 80 79 78 77 76 75 74 73 72 71
 61 62 63 64 65 66 67 68 69 70
 60 59 58 57 56 55 54 53 52 51
 41 42 43 44 45 46 47 48 49 50
 40 39 38 37 36 35 34 33 32 31
 21 22 23 24 25 26 27 28 29 30
 20 19 18 17 16 15 14 13 12 11
  1  2  3  4  5  6  7  8  9 10
\$\endgroup\$
2
  • \$\begingroup\$ Yep that was pretty obvious. I started to fight to get the right output with this && statements and then after some time I didn't see this unnecessary double check ^^ \$\endgroup\$
    – Cats
    Commented Sep 7 at 9:44
  • \$\begingroup\$ @Cats: We've all been there... we're so relieved we got the code to work that we don't think to see if we can simplify it. That's why we have code reviews and CodeReview :) \$\endgroup\$
    – toolic
    Commented Sep 7 at 10:40
4
\$\begingroup\$

Arrays are indexed starting from 0 in C, which means that valid indices for buffer are 0..99. This code overflows buffer by accessing buffer[100]. This can and will cause strange and hard to debug misbehaviour in programs and remote-code-execution vulnerabilities in the worst case.

Depending on the compiler you are using, there may be a setting that can help catch these sort of bugs. For example, GCC has -fstack-protector-all.

\$\endgroup\$
1
  • \$\begingroup\$ I fixed that. The interesting thing is, I did discover that while I tried to debug. When I was building the project and run the file there was no error/warning. \$\endgroup\$
    – Cats
    Commented Sep 7 at 18:36
3
\$\begingroup\$

As @toolic said, You can use "%3d" to print the number on a 3 width field, aligning every one.

In addition

You can completely remove the if that are in the two k loop. For that, you just need to decrease the loop's iteration per one and moving both the printf and the boolean change outside of the loop. Per example, for the odd clause, instead of

for (int8_t k = 9; k > -1; k--) {
  if (((k % 10) == 0) && (k != 10)) {
    printf("%d ", board[i]);
    printf("\n");
    row_even = true;
    continue;
  }
  printf("%d ", board[i - k]);
}

You can write that

for (int8_t k = 9; k > 0; k--) {
  printf("%3d", board[i - k]);
}
printf("%3d\n", board[i]);
row_even = true;

As you can see, I combine the 2 printf located in the if statement in a single printf, it's a little thing but in that way you can avoid a syscall (printf is not a syscall but, under the hood, it use write, which is a syscall)

\$\endgroup\$
1
  • \$\begingroup\$ Thank you very much, this also helped me very much. I also did not see this simple solution! \$\endgroup\$
    – Cats
    Commented Sep 7 at 18:35
2
\$\begingroup\$

Three major improvements are:

  1. try to avoid using continue as much as possible. It is rarely needed, and obfuscates the logic. It is not needed at all in your case.
  2. the newline character needs to be printed at the end of each line. That is logically after the inner loop is finished, therefore also place it there in your logic. Doing that also avoids the need for the if inside the loop.
  3. avoid writing the same code twice. Printing the number is only needed inside the inner loops, so don't also print it after an inner loop is finished.

Putting that together, you get the first inner loop as:

        for (uint8_t k = 0; k < 10; k++) {
            printf("%3d ", board[i - k]);
        }
        printf("\n");
\$\endgroup\$
1
\$\begingroup\$

Bug

uint8_t board[100]; ... for (uint8_t i = 1; i < 101; i++) { board[i] = i; } attempts to access board[100] which is out of the valid 0 - 99 range.

Better as

  #define BOARD_SIZE 100
  uint8_t board[BOARD_SIZE];
  ...
  for (size_t i = 0; i < BOARD_SIZE; i++) {
    board[i] = i + 1;
  }

[Edit]
Now I see this point lightly covered by TrayMan. Moving answer to wiki.

\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.