Creating Layout of 2D Board game

Question

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 

Answer 1

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 

Answer 2

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.

Answer 3

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)

Answer 4

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"); 

Answer 5

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.