Я сделал эту простую игру в тетрис, в которой у меня есть только 2 блока: квадрат и линия. Также игра заканчивается после того, как будет создано 20 блоков и игрок выиграет. Мне было интересно, как я могу улучшить его в целом.
Вот код:
#include <stdio.h>
#include <windows.h>
#include <string.h>
#include <time.h>
#define RIGA 21
#define COLONNA 10
#define DESTRA 77
#define SINISTRA 75
#define GIU 80
#define SU 72
void GotoXY(int x, int y)
{
COORD CursorPos = {x, y};
HANDLE hConsole = GetStdHandle(STD_OUTPUT_HANDLE);
SetConsoleCursorPosition(hConsole, CursorPos);
}
// FUNCTION PER RALLENTARE I MOVIMENTI
void ritardo()
{
long double i;
for(i=0;i<=(30000000);i++);
}
int i, j; // CONTATORI
int sposta, random; // VARIABILI BOOLEANE
int punti = 0; // VARIABILE PER ASSEGNAZIONE DEI PUNTI
int contatore_pezzi = 1; // VARIABILE PER CONTARE IL NUMERO DI OGGETTI CREATI
char griglia[21][10]; // DICHIARAZIONE ARRAY DI CHAR
char pezzo[2][2] = {{'X', 'X'}, // DICHIARAZIONE CUBO
{'X', 'X'}};
char linea[3][1] = {{'X'},
{'X'},
{'X'}}; // DICHIARAZIONE LINEA
char linea_or[1][3] = { {'X'},{'X'},{'X'}};
void caricaArray() // INIZIALIZZA L'ARRAY VUOTO CON CICLI FOR INNESTATI
{
for (i = 0; i < RIGA; i++)
{
for (j = 0; j < COLONNA; j++)
{
griglia[i][j] = ' ';
}
}
}
// PROTOTIPI FUCTION
void movimenti();
void assegnaPezzo();
void eliminaRiga();
void stampaBordi();
int main()
{
caricaArray();
movimenti();
printf("nn");
return 0;
}
void visualizzaArray() // VISUALIZZA L'ARRAY ALLO STATO ATTUALE
{
printf("n");
for (i = 0; i < RIGA; i++)
{
printf(" "); //Spazio per visualizzare tutti i punti all'interno del bordo
for (j = 0; j < COLONNA; j++)
{
printf("%c ", griglia[i][j]);
}
printf("n");
}
stampaBordi();
}
void movimenti() // GESTISCE TUTTI I POSSIBILI MOVIMENTI DELLA FORMA
{
do {
eliminaRiga(); // Ad ogni ciclo verifica la presenza di righe piene
int r = 0;
int c = 4;
srand(time(NULL)); // Random
random = rand() % 2;
assegnaPezzo();
visualizzaArray();
do {
GotoXY(r,c); // Inizia a scorrere verso il basso
r++;
system("cls");
if (random == 0) { // Se l'oggetto creato e' il cubo
griglia[r-1][c] = ' '; // Libera le posizioni precedenti
griglia[r-1][c+1] = ' ';
griglia[r][c] = pezzo[0][0]; // Riassegna l'oggetto nella nuova posizione
griglia[r][c+1] = pezzo[0][1];
griglia[r+1][c] = pezzo[1][0];
griglia[r+1][c+1] = pezzo[1][1];
}
if (random == 1) { // Se l'oggetto creato e' la linea
if ( !kbhit() ) // Se non si premono tasti
{
griglia[r-1][c] = ' '; // Libera le posizioni precedenti
griglia[r][c] = linea[0][0]; // Riassegna l'oggetto nella nuova posizione
griglia[r+1][c] = linea [1][0];
griglia[r+2][c] = linea [2][0];
}
}
visualizzaArray();
ritardo();
if ( kbhit() ) // Se durante lo scroll verticale si preme un tasto
{
sposta = getch(); // Prende in input un tasto e lo assegna alla variabile sposta
if (sposta==SU) // E se e' la freccia SU
{
griglia[r-1][c] = ' ';
griglia[r][c-1] = linea_or[0][0];
griglia[r][c] = linea_or[1][0];
griglia[r][c+1] = linea_or[2][0];
}
if (sposta == DESTRA ) // E se e' la freccia DESTRA
{
c++; // L'oggetto si sposta a destra
system("cls");
if (random == 0) {
if (c+1>COLONNA-1) { break; } // Se supera il bordo destro
griglia[r][c-1] = ' '; // Libera le posizioni precedenti
griglia[r+1][c-1] = ' ';
}
else if (random == 1) {
griglia[r-2][c-1] = ' '; // Libera le posizioni precedenti
griglia[r-1][c-1] = ' ';
griglia[r][c-1] = ' ';
griglia[r+1][c-1] = ' ';
griglia[r+2][c-1] = ' ';
}
visualizzaArray(); //visualizza il tutto
}
if (sposta == SINISTRA ) // E se e' la freccia SINISTRA
{
c--; // L'oggetto si sposta a sinistra
if (c<0) { break; } // Se supera il bordo sinistro
system("cls");
if (random == 0) {
griglia[r][c+2] = ' '; // Libera le posizioni precedenti
griglia[r+1][c+2] = ' ';
}
else if (random == 1) {
griglia[r-1][c+2] = ' '; // Libera le posizioni precedenti
griglia[r-1][c+1] = ' ';
griglia[r][c+1] = ' ';
griglia[r+1][c+1] = ' ';
griglia[r+2][c+1] = ' ';
}
visualizzaArray();
}
}
if (random == 0) {
if (griglia[r+2][c] == 'X' || griglia[r+2][c+1] == 'X') // Se il pezzo tocca un altro pezzo inferiormente
{
if (griglia[0][c] == 'X' || griglia[1][c] == 'X') // Se lo spazio superiore e' troppo piccolo
{
system("cls");
printf("HAI PERSO!nPremi ESC per uscire.");
break;
}
contatore_pezzi++;
return movimenti(); //Allora attiva la ricorsivita'
}
}
if (random == 1) {
if (griglia[r+3][c] == 'X') {
if (griglia[0][c] == 'X' || griglia[1][c] == 'X')
{
system("cls");
printf("HAI PERSO!nPremi ESC per uscire.");
break;
}
contatore_pezzi++;
return movimenti();
}
}
if (random == 0) {
if (r+1==20) { // Se tocca il borso inferiore crea un altro oggetto
contatore_pezzi++;
return movimenti(); }
}
else if (random == 1) {
if (r+2==20) { // Se tocca il borso inferiore crea un altro oggetto
contatore_pezzi++;
return movimenti(); }
}
} while ( sposta != 27 );
} while ( getch() != 27 );
}
void stampaBordi() // STAMPA LA CORNICE DEL CAMPO DI GIOCO
{
for(i=1;i<=19;i++)
{
GotoXY(i,22); putch('-');
GotoXY(i,0); putch('-');
}
printf(" PUNTI:%d - PEZZO No %d", punti, contatore_pezzi);
for (j=1;j<22; j++) {
GotoXY(0,j); putch('|');
GotoXY(20,j); putch('|');
}
}
void assegnaPezzo()
{
int i1 = 0;
int j1 = 4;
if (random==0) {
griglia[i1][j1] = pezzo[0][0]; // Assegna il pezzo nella posizione iniziale
griglia[i1][j1+1] = pezzo[0][1];
griglia[i1+1][j1] = pezzo[1][0];
griglia[i1+1][j1+1] = pezzo[1][1];
}
if (random==1) {
griglia[i1][j1] = linea[0][0];
griglia[i1+1][j1] = linea [1][0];
griglia[i1+2][j1] = linea [2][0];
}
}
void eliminaRiga() // ELIMINA UNA O PIU' RIGHE, SE PIENE
{
int k;
int cont_riga;
int colonna;
char aus[COLONNA];
for (cont_riga=0; cont_riga<=20; cont_riga++)
{
if (griglia[cont_riga][0] == 'X' && griglia[cont_riga][1] == 'X' && griglia[cont_riga][2] == 'X' && griglia[cont_riga][3] == 'X' && griglia[cont_riga][4] == 'X' && griglia[cont_riga][5] == 'X' && griglia[cont_riga][6] == 'X' && griglia[cont_riga][7] == 'X' && griglia[20][8] == 'X' && griglia[20][9] == 'X')
{
for (colonna=0; colonna<10; colonna++) {
aus[colonna] = griglia[cont_riga-1][colonna];
}
for (colonna=0; colonna<10; colonna++) {
griglia[cont_riga][colonna] = aus[colonna];
}
for (colonna=0; colonna<10; colonna++) {
griglia[cont_riga-1][colonna] = '.';
}
punti++;
for (k=2; k<11; k++) {
if (griglia[cont_riga-k][0] == 'X' || griglia[cont_riga-k][1] == 'X' || griglia[cont_riga-k][2] == 'X' || griglia[cont_riga-k][3] == 'X' || griglia[cont_riga-k][4] == 'X' || griglia[cont_riga-k][5] == 'X' || griglia[cont_riga-k][6] == 'X' || griglia[cont_riga-k][7] == 'X' || griglia[cont_riga-k][8] == 'X' || griglia[cont_riga-k][9] == 'X' )
{
for (colonna=0; colonna<10; colonna++) {
aus[colonna] = griglia[cont_riga-k][colonna];
}
for (colonna=0; colonna<10; colonna++) {
griglia[cont_riga-k+1][colonna] = aus[colonna];
}
for (colonna=0; colonna<10; colonna++) {
griglia[cont_riga-k][colonna] = '.';
}
}
}
system("cls");
visualizzaArray();
}
}
}
3 ответа
Всегда писать исходный код и комментарии на английском языке (я тоже не носитель английского языка). Рано или поздно вам понадобится поделиться кодом с другими людьми по всему миру, как вы только что сделали здесь и сейчас. Поэтому он должен быть на английском языке.
Что еще более важно, изучение двух технических терминов программирования для всего, когда вам нужно выучить только один, — это безумие. Худшая ошибка, которую я совершил, изучая инженерное дело в университете, — это читать книги, переведенные на мой родной язык, а не на английский. Это означало, что мне пришлось выучить в два раза больше технических терминов: настоящий, правильный термин на английском языке и бессмысленный термин, сомнительно переведенный на мой родной язык. Последнее — совершенно бесполезное знание — в реальной жизни вне школы используются английские термины.
Как указано в другом обзоре,
ritardo
busy-delay следовало заменить на Windows APISleep
в таком случае. В более продвинутом программировании Windows с потоками и событиями вы бы не использовалиSleep
либо, ноWaitForSingleObject
и т. д. Многопоточность — это довольно сложная тема, но вам, в конце концов, придется заняться. Затем эта программа будет использовать один поток для графики, один для ввода и, возможно, третий для вычислений. Вместо использования старого стиля MS DOSkbhit
— В DOS не было потоков.если ты мы для реализации цикла задержки занятости итератор цикла
i
нужно будет объявитьvolatile
. В противном случае компилятор просто оптимизирует весь цикл.Никогда не используйте числа с плавающей запятой для итераторов цикла. Это замедляет выполнение кода, и выражение сравнения может неожиданно завершиться ошибкой из-за неточности с плавающей запятой.
Всегда пытайтесь объявить итератор цикла внутри цикла for:
for(int i=0; ...
.Вам нужно избавиться от всех глобальных переменных. Много информации о том, почему они плохи, чтобы их можно было найти в сети.
Никогда не используйте устаревшие функции пустых скобок в C:
void caricaArray()
. Это означает, что функция принимает Любые параметр, и этот стиль может быть удален из C в любой момент, поскольку он формально устарел. Правильная формаvoid caricaArray(void)
.(Не путать с C ++, где пустые скобки допустимы и эквивалентны
(void)
.)Как отмечалось в другом обзоре, вам нужно выбрать стиль кодирования и придерживаться его. Размещение скобок, отступ и т. Д. В существующем виде код почти не читается.
srand();
следует вызывать только один раз в начале программы, вы вызываете его внутри цикла.
Некоторые предложения в произвольном порядке:
Организуйте свои файлы заголовков.
Мне нравится группировать связанные вещи вместе. В данном случае «стандарт» или «зависит от ОС». Я также по умолчанию сортирую вещи в алфавитном / асциибетическом порядке, если нет причин поступать иначе.
Сгруппируйте связанные вещи: функции с функциями, переменные с переменными. Не смешивайте функции и глобальные переменные случайным образом.
Не делайте переменные глобальными без необходимости. Ваш
i
иj
должны быть локальными переменными.Линии тетриса состоят из 4 блоков, а не 3.
По возможности используйте библиотечные функции. Ваш
ritardo()
кажется плохой заменой POSIXsleep()
или WindowsSleep()
.Использовать
memset
для инициализации вашегоgriglia
вcaricaArray
:memset(griglia, SPACE, sizeof(griglia));
Стандарт C требует использования
void
для создания прототипа функции без аргументов. (В C ++ пустые скобки могут означать отсутствие аргументов.) Итак, строка вроде:void movimenti();
на самом деле не прототип, а декларация.
Включи всевозможные предупреждения. В вашем компиляторе есть где-то настройки, которые будут включать «Все предупреждения». Тогда у него, вероятно, есть настройки, которые будут включать «Необычные предупреждения» или «Дополнительные предупреждения» или что-то в этом роде. Он может даже иметь настройку «Включить невероятно педантичные, нелепые предупреждения». Включите их. Все они.
Определение
main()
должна быть либо самой первой, либо самой последней функцией. Если вы предпочитаете «читать мои программы», тоmain
является первым, и функции будут добавляться в конец по мере их использования (основные вызовы A, A вызовы B и т. д.). Если вы предпочитаете «избегать предварительных объявлений», вы пытаетесь определить каждую функцию до того, как она будет вызвана. Таким образом, функции нижнего уровня идут вверху файла, затем выше и, наконец,main()
в конце. Если вы предпочитаете «все должно быть в алфавитном порядке», то вы психопат, но, по крайней мере, знаете, где все должно быть.Это вопрос стиля, поэтому выберите один и придерживайтесь его.
Не смешивайте режимы терминала. Если вы собираетесь позиционировать курсор и рисовать символы, напишите свои собственные функции печати (для управления окном прокрутки, если ничего другого). Также не звоните
cls
чтобы очистить экран. Просто напишите свою собственную функцию.Напишите больше функций
У вас есть очень большие функции. Существует множество рекомендаций о том, какой размер подходит для функции, но большинство из них сосредоточено на том, чтобы функции оставались видимыми на одном экране. (Некоторые предлагают очень низкие числа. Вы удивитесь, что вы можете сделать, если попытаетесь использовать действительно небольшое число!)
В качестве предложения, если вы делаете «одно и то же» в разных местах, это должна быть функция.
Если вы делаете одно и то же по-разному — например, рисуете квадрат вместо рисования линии — тогда вам, вероятно, нужно сесть и подумать, как вы могли бы относиться к ним так же. Возможно, вам нужна структура данных, чтобы узнать, каковы размеры? Или, может быть, вы могли бы использовать указатель и одно целое число, чтобы сказать вам, на что указывает? An
enum
тип?Кешировать результаты
kbhit()
. Эта функция проверяет, была ли нажата клавиша. Это означает, что каждый раз, когда вы звоните, вы делаете еще одну проверку. В цикле, вероятно, проще вызвать его один раз в верхней части цикла, запомнить результат и дождаться следующего прохода цикла, чтобы вызвать его в другой раз.Выберите стиль программирования и оставайся с ней до конца программы! Я вижу три стили отступов в вашей программе (все рядом друг с другом!):
if (sposta==SU) // E se e' la freccia SU { griglia[r-1][c] = ' '; griglia[r][c-1] = linea_or[0][0]; griglia[r][c] = linea_or[1][0]; griglia[r][c+1] = linea_or[2][0]; }
Это «стиль GNU». Не используйте его, если вы лично не должны Ричарду Столмену денег.
if (sposta == DESTRA ) // E se e' la freccia DESTRA {
Это «стиль Оллмана».
c++; // L'oggetto si sposta a destra system("cls"); if (random == 0) {
Это «стиль One True Brace» или «стиль K&R» в зависимости от того, как вы относитесь к функциям.
if (c+1>COLONNA-1) { break; } // Se supera il bordo destro
Это «вариант обязательных скобок», также известный как «Баннерный самолет» стиль.
Так что просто выберите один из GNU, Allman, K&R или BannerPlane. Моя ошибка, четыре стиля.
- Использовать по имени
#define
s илиenum
константы везде. У вас хорошее начало, но потом вы увязнете в таких ценностях, как'X'
и20
. Что такое 20?
Вероятно, есть что сказать, но на данный момент этого достаточно.
В целом, похоже, неплохой процедурный код.
Объем
int i, j; // CONTATORI
Эти индексы могут быть локальными для функций. Глобалы — это нормально, но не в этом случае.
Также sposta
вероятно всегда местный для movimenti()
функция.
Магические числа
void stampaBordi() // STAMPA LA CORNICE DEL CAMPO DI GIOCO
{
for(i=1;i<=19;i++)
{
GotoXY(i,22); putch('-');
GotoXY(i,0); putch('-');
- Я не знаю, что означают эти числа (19, 22), и вы, вероятно, не узнаете через несколько дней / месяцев / лет
- что
i
такое columnIndex или numberOfIterations, или что-то еще?
Использование структур
char griglia[21][10]; // DICHIARAZIONE ARRAY DI CHAR
...
void stampaBordi() // STAMPA LA CORNICE DEL CAMPO DI GIOCO
{
for(i=1;i<=19;i++)
{
GotoXY(i,22); putch('-');
GotoXY(i,0); putch('-');
Все это связано с доской. Обычно это хорошая идея собрать эти переменные вместе, потому что легче подумать, когда связаны связанные значения вместе (причина понимания / удобочитаемости).
struct Grid {
char cells[21][10];
int rows;
int columns;
};
Grid grid = {
rows = 21;
columns = 10;
}
...
void stampaBordi(Grid * grid) // STAMPA LA CORNICE DEL CAMPO DI GIOCO
{
for(int i=1;i<= grid->rows - 2;i++)
{
GotoXY(i, grid->rows + 1 ); putch('-');
GotoXY(i, 0 ); putch('-');
Я почти уверен, что некоторые из этих констант также должны быть членами структуры Grid 🙂 …
#define RIGA 21
#define COLONNA 10
#define DESTRA 77
#define SINISTRA 75
#define GIU 80
#define SU 72
Сжатие кода
Также в этом случае …
if (sposta == DESTRA ) // E se e' la freccia DESTRA
{
c++; // L'oggetto si sposta a destra
system("cls");
if (random == 0) {
if (c+1>COLONNA-1) { break; } // Se supera il bordo destro
griglia[r][c-1] = ' '; // Libera le posizioni precedenti
griglia[r+1][c-1] = ' ';
}
else if (random == 1) {
griglia[r-2][c-1] = ' '; // Libera le posizioni precedenti
griglia[r-1][c-1] = ' ';
griglia[r][c-1] = ' ';
griglia[r+1][c-1] = ' ';
griglia[r+2][c-1] = ' ';
}
visualizzaArray(); //visualizza il tutto
}
Вытягивайте связанную информацию в структуры, чтобы облегчить создание новых материалов и чтение.
// code like this repeats on multiple places
// and it seems like You are working with an array of offsets
// and You are doing single operation to them
griglia[r-2][c-1] = ' '; // Libera le posizioni precedenti
griglia[r-1][c-1] = ' ';
griglia[r][c-1] = ' ';
griglia[r+1][c-1] = ' ';
griglia[r+2][c-1] = ' ';
// this can be compressed like this
struct Offset { int x; int y; }
Offset offsets = [{-2,-1},{-1,-1},{0,-1},{1,-1},{2,-1}];
int offsetCount = 5;
for(auto i = 0; i < offsetCount; i++) {
auto offset = offsets[i];
griglia[r + offset.x][c + offset.y] = ' ';
}
// and it can be pulled into function
clearOffsets( Offsets * offsets, int offsetCount )
// Also these offsets seem to be always asociated with a tetromino
// each tetromino has it's own offsets, so
struct Offset { int x; int y; }
struct Tetromino { Offset cellOffsets[]; int offsetCount; ... }
Tetromino l_shape = { cellOffsets = [{-2,-1},{-1,-1},{0,-1},{1,-1},{2,-1}], 5}
// now You can make functions like this
clearTetromino(l_shape);
rotateTetrominoLeft(l_shape);
// also You can reuse function to rotate another shape or shapes ...
rotateTetrominoLeft(box_shape);
rotateTetrominoRight(shapes[shapeIndex]);
Подробнее о сжатии кода см. Отлично https://caseymuratori.com/blog_0015 .
Форматирование
else if (random == 1) {
if (r+2==20) { // Se tocca il borso inferiore crea un altro oggetto
Мне сложно быстро это сканировать, потому что я привык к стилю скобок K&R и стилей скобок Allman (большинство людей их используют).
Комментарии
Английский был бы хорош в комментариях 🙂
У меня есть такая эвристика: я избегаю комментариев, объясняющих, что означает переменная или функция, вместо этого я делаю лучшее (более длинное, более информативное) имя для функций / переменных. Я комментирую неочевидные вещи …
Это нормально
if (r+2==20) { // Se tocca il borso inferiore crea un altro oggetto
но, опять же, возможно, удаление комментария путем введения переменной может прояснить ситуацию (и вы можете повторно использовать переменную в нескольких местах)
auto touchesLongerEdge = r + 2 === 20;
Верно, что сокращение области, в которой находится переменная, упрощает понимание кода, но некоторые переменные являются глобальными по своей природе. Имея цель избавиться от все globals — это ерунда.
— user712092
@ user712092 Как обычно, это зависит от того, что вы имеете в виду под словом «глобальный». Переменные области внутренней связи файла в порядке. Спагетти с внешней связью между несколькими файлами не годятся.
— Лундин
@ user712092 Я считаю, что, особенно по мере роста программы, вероятность переменная (а не просто константа) поистине глобальное исчезновение. Проведение четких линий вокруг того, какие данные необходимы там, где это хорошо для качества кода, и то же самое касается ослабления связи путем программирования с абстракциями (например, с параметрами, а не с закрытыми глобальными объектами). Конечно, всегда можно зайти слишком далеко, и это ситуативный вопрос.
— Резиновая утка