Текстовый Minesweeper на C

Это моя первая программа на C. Код работает, но я еще не умею писать идиоматический C. Как мне это улучшить?

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#define UNCLICKED 0
#define CLICKED 1
#define FLAGGED 2

#define NOT_BOMB 0
#define BOMB 1

struct piece {
    char user_state, game_state, value;
};

int generate_rand();
int valid(int, int);
void print_board(struct piece[8][8], int);
int click(struct piece[8][8], int, int);
int game_over(struct piece[8][8]);

int main() {
    struct piece board[8][8];

    int i, j;
    for (i = 0; i < 8; i++) {
        for (j = 0; j < 8; j++) {
            board[i][j].user_state = UNCLICKED;
            board[i][j].game_state = NOT_BOMB;
            board[i][j].value = 0;
        }
    }

    srand(time(0));

    int num_bombs = 10;

    while (num_bombs > 0) {

        int y, x;

        do {
            y = generate_rand();
            x = generate_rand();
        } while (board[y][x].game_state == BOMB);

        board[y][x].game_state = BOMB;

        int delta_row, delta_column;
        for (delta_row = -1; delta_row <= 1; delta_row++) {
            for (delta_column = -1; delta_column <= 1; delta_column++) {
                if (valid(y + delta_row, x + delta_column)) {
                    board[y + delta_row][x + delta_column].value++;
                }
            }
        }

        num_bombs--;
    }

    print_board(board, 1);

    while (1) {
        char row, column, move_type;
        printf("nEnter a row: ");
        row = getchar() - 48;
        getchar();
        printf("Enter a column: ");
        column = getchar() - 48;
        getchar();

        if (!valid(row, column)) {
            printf("Location is not on board. Try again: n");
            continue;
        }

        if (board[row][column].user_state == CLICKED) {
            printf("That square is already open. Try again: n");
            continue;
        }

        printf("[C]lick or [F]lag? ");
        move_type = getchar();
        getchar();
        printf("n");

        if (move_type == 'C') {
            if (click(board, row, column)) {
                printf("That was a bomb! n");
                print_board(board, 0);
                break;
            }
        } else if (move_type == 'F') {
            if (board[row][column].user_state == CLICKED) {
                printf("That square is already open. Try again: n");
                continue;
            } else {
                board[row][column].user_state = FLAGGED;
            }
        } else {
            printf("Enter either 'C' or 'F'. Try again: n");
            continue;
        }

        if (game_over(board)) {
            printf("You win! n");
            print_board(board, 0);
            break;
        } else {
            print_board(board, 1);
        }
    }

    return 0;
}

int generate_rand() {
    return (rand() % 8);
}

int valid(int row, int column) {
    return row >= 0 && row < 8 && column >= 0 && column < 8;
}

void print_board(struct piece board[8][8], int in_game) {
    int i, j;
    for (i = 0; i < 8; i++) {
        for (j = 0; j < 8; j++) {
            if (in_game) {
                if (board[i][j].user_state == CLICKED) {
                    printf("%d ", board[i][j].value);
                } else if (board[i][j].user_state == FLAGGED) {
                    printf("F ");
                } else {
                    printf("- ");
                }
            } else {
                if (board[i][j].game_state == NOT_BOMB) {
                    if (board[i][j].user_state == FLAGGED) {
                        printf("X ");
                    } else {
                        printf("%d ", board[i][j].value);
                    }
                } else {
                    printf("B ");
                }
            }
        }
        printf("n");
    }
}

int click(struct piece board[8][8], int row, int column) {
    if (!valid(row, column) || board[row][column].game_state == BOMB || board[row][column].user_state == CLICKED) {
        return 1;
    }

    board[row][column].user_state = CLICKED;
    if (board[row][column].value == 0) {
        int i, j;
        for (i = -1; i <= 1; i++) {
            for (j = -1; j <= 1; j++) {
                click(board, row + i, column + j);
            }
        }
    }

    return 0;
}

int game_over(struct piece board[8][8]) {
    int i, j;
    for (i = 0; i < 8; i++) {
        for (j = 0; j < 8; j++) {
            if (board[i][j].game_state == NOT_BOMB && board[i][j].user_state == UNCLICKED) {
                return 0;
            }
        }
    }

    return 1;
}

Имеет ли смысл поместить часть кода в отдельный файл заголовка, или это слишком усложняет ситуацию?

2 ответа
2

Меня сразу бросает в глаза то, что мы проезжаем struct piece board[8][8] вокруг довольно много. Есть две вещи, которые я бы здесь изменил. Первый — использовать typedef (или, возможно, небольшую структуру), чтобы сохранить всю эту типизацию. Другой — дать имена размерам.

#define BOARD_WIDTH  8
#define BOARD_HEIGHT 8

typedef struct piece game_board[BOARD_HEIGHT][BOARD_WIDTH];

Если мы хотим построить игру для другой высоты и ширины, тогда будет только одно место для изменения (как только границы цикла будут настроены для использования определенных макросов вместо буквальных 8). И мы упростили его, если мы когда-нибудь захотим в будущем перейти на размер платы, выбранный во время выполнения (это более сложное упражнение, которое вы должны попробовать, когда почувствуете, что к нему готовы).

Typedef означает, что вместо того, чтобы писать аргументы функции, такие как

void print_board(const struct piece board[BOARD_HEIGHT][BOARD_WIDTH], int in_game);

они могут быть короче и понятнее:

void print_board(const game_board board, int in_game);

Здесь есть слабые места:

    printf("nEnter a row: ");
    row = getchar() - 48;
    getchar();
    printf("Enter a column: ");
    column = getchar() - 48;
    getchar();

Помните, что любой звонок getchar() может вернуться EOF. Если это произойдет, мы, вероятно, захотим завершить программу, поскольку с этого момента мы не можем ожидать каких-либо полезных данных.

Во-вторых, магическое число 48 зависит от используемой кодировки символов — мы можем сделать программу более переносимой, используя '0' как очевидно задумано.

Это также довольно хрупко — мы ожидаем одного символа и новой строки, но пользователи делают ошибки и могут вводить несколько символов (и мы, вероятно, захотим разрешить это, если сделаем доску больше 10✕10).

К счастью, у нас есть лучшая альтернатива посимвольному вводу, и это форматированный ввод с использованием scanf(). Это будет выглядеть примерно так

    printf("nEnter a row and column: ");
    while (scanf("%hhd%hhd", &row, &column) != 2) {
        if (feof(stdin)) {
            /* scanf can never succeed now */
            fputs("Input failuren", stderr);
            return EXIT_FAILURE;
        }
        /* Discard any remaining input */
        scanf("%*[^n]");   /* ignore errors here */
        printf("Invalid input.  Enter a row and column: ");
    }

Это не на 100% надежно, но это улучшение.


Небольшое исправление: эти объявления функций должны быть прототипы, т.е. укажите, какие аргументы они принимают:

int generate_rand();

int main() {

Мы сообщаем компилятору, что они вообще не принимают аргументов (а не неуказанных аргументов), — это написать void в скобках:

int generate_rand(void);

int main(void) {

Структура немного разбалансирована. Есть несколько низкоуровневых функций, но большая часть логики заключена в огромном main(). Попробуйте разделить его на высокоуровневые функции — возможно initialise_board() может быть одним из них?


Есть еще кое-что, что можно улучшить, но я надеюсь, что это дает вам некоторые моменты, которые следует учитывать.

  • Что значит typedef struct piece game_board[BOARD_HEIGHT][BOARD_WIDTH]; делать?

    — ап

  • @ap Он создает тип, который вы можете использовать, и это уменьшает количество кода, необходимого для объявления функций.

    — pacmaninbw


  • 1

    @ap: Да, как говорит pacmaninbw, это делает код короче и понятнее — см. последнее изменение.

    — Тоби Спейт

  • 1

    @ap см. en.cppreference.com/w/c/language/typedef и добавьте этот сайт в закладки, чтобы вы могли легко найти информацию в будущем.

    — JDłuosz

Некоторые предложения по стилю:

  1. Используйте перечисления вместо #defines.

    enum {
      state_UNCLICKED,
      state_CLICKED,
      state_FLAGGED
    } ;
    

С последующим if (board[i][j].user_state == state_CLICKED)

  1. Определения структуры легче понять, если каждый член находится на отдельной строке.
struct piece {
   char user_state;
   char game_state;
   char  value;
};
  1. Как правило, удобно помещать имена аргументов в объявления функций. Тогда читателям не нужно переходить к реализации, чтобы узнать, что означают аргументы.

  2. Я мог бы провести рефакторинг

           board[i][j].user_state = UNCLICKED;
           board[i][j].game_state = NOT_BOMB;
           board[i][j].value = 0;

в board_square_init(board, i,j) функция. Я мог бы также создать square_place_bomb(board, i,j) функция, которая выполняет обновление соседа.

  1. Это странно, что click возвращается 1 по нескольким не связанным между собой вещам: за пределами поля, уже нажали и бомба. Вам это сходит с рук, потому что вы предварительно тестируете нажатие перед вызовом этой функции, но было бы чище, если бы вы вернули другое значение для разных сбоев. Может быть, добавить state_INVALID к перечислению в комментарии 1 и вернуть член перечисления.

  2. Многим не нравятся глобальные переменные в C, и для этого есть несколько веских причин. Но если вы не планируете писать программу, которая управляет несколькими досками одновременно, вы можете просто объявить единую доску один раз глобально и перестать передавать ее каждой отдельной функции. Сегодня вы обмениваете потенциально полезную возможность поддержки нескольких плат в будущем на более простой код.

    Добавить комментарий

    Ваш адрес email не будет опубликован. Обязательные поля помечены *