Это моя первая программа на 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 ответа
Меня сразу бросает в глаза то, что мы проезжаем 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()
может быть одним из них?
Есть еще кое-что, что можно улучшить, но я надеюсь, что это дает вам некоторые моменты, которые следует учитывать.
Некоторые предложения по стилю:
Используйте перечисления вместо #defines.
enum { state_UNCLICKED, state_CLICKED, state_FLAGGED } ;
С последующим if (board[i][j].user_state == state_CLICKED)
- Определения структуры легче понять, если каждый член находится на отдельной строке.
struct piece {
char user_state;
char game_state;
char value;
};
Как правило, удобно помещать имена аргументов в объявления функций. Тогда читателям не нужно переходить к реализации, чтобы узнать, что означают аргументы.
Я мог бы провести рефакторинг
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)
функция, которая выполняет обновление соседа.
Это странно, что
click
возвращается1
по нескольким не связанным между собой вещам: за пределами поля, уже нажали и бомба. Вам это сходит с рук, потому что вы предварительно тестируете нажатие перед вызовом этой функции, но было бы чище, если бы вы вернули другое значение для разных сбоев. Может быть, добавитьstate_INVALID
к перечислению в комментарии 1 и вернуть член перечисления.Многим не нравятся глобальные переменные в C, и для этого есть несколько веских причин. Но если вы не планируете писать программу, которая управляет несколькими досками одновременно, вы можете просто объявить единую доску один раз глобально и перестать передавать ее каждой отдельной функции. Сегодня вы обмениваете потенциально полезную возможность поддержки нескольких плат в будущем на более простой код.
Что значит
typedef struct piece game_board[BOARD_HEIGHT][BOARD_WIDTH];
делать?— ап
@ap Он создает тип, который вы можете использовать, и это уменьшает количество кода, необходимого для объявления функций.
— pacmaninbw
@ap: Да, как говорит pacmaninbw, это делает код короче и понятнее — см. последнее изменение.
— Тоби Спейт
@ap см. en.cppreference.com/w/c/language/typedef и добавьте этот сайт в закладки, чтобы вы могли легко найти информацию в будущем.
— JDłuosz