Форум программистов, компьютерный форум CyberForum.ru

Оцените код "Змейки" - C++

Восстановить пароль Регистрация
 
Aspirin
29 / 6 / 0
Регистрация: 05.10.2012
Сообщений: 117
15.08.2014, 16:16     Оцените код "Змейки" #1
Прошу добрых жителей форума оценить мою "Змейку". Посоветуйте на данном примере, как не следует писать и как лучше. Если есть конкретные замечания по коду, по возможности опишите, как следовало бы сделать. Принимаю любую критику.
C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
//Constants.h
#ifndef CONSTANTS_H
#define CONSTANTS_H
 
const int n = 16;                   //number columns
const int m = 16;                   //number rows
const int size_cell = 30;           
const int width = n*size_cell;      //width of window
const int height = m*size_cell;     //height of window
 
struct coordinates
{
    int x;
    int y;
};
 
 
 
#endif CONTSANTS_H
C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
//Food.h
#ifndef FOOD_H
#define FOOD_H
 
class Food
{
public:
    void spawn();
    void draw();
    int x;
    int y;
};
 
 
#endif FOOD_H
C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
//Food.cpp
#include <glut.h>
#include <cstdlib>
#include <time.h>
 
#include "Constants.h"
#include "Food.h"
#include "Snake.h"
 
extern Snake snake;
 
void Food::spawn()
{
    srand(time(NULL));
    x = (rand() % n)*size_cell;
    y = (rand() % m)*size_cell;
    for (int i = 0; i < snake.getSize(); i++)
    {
        if ((snake.block[i].x == x) && (snake.block[i].y == y))
            spawn();
    }
}
 
void Food::draw()
{
    glColor3f(1.0, 0.0, 0.0);
    glRectd(x + 1, y + 1, x + size_cell, y + size_cell);
}
C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
//Snake.h
#include "Constants.h"
 
#ifndef SNAKE_H
#define SNAKE_H
 
class Snake
{
public:
    coordinates block[width*height];
    Snake();
    void draw();
    void move();
    void setDirection(int dir);
    void outside_window();
    int getSize();
    void eat();
    void cut();
private:
    int size;
    int direction;
};
 
 
#endif SNAKE_H
C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
//Snake.cpp
#include <glut.h>
 
#include "Snake.h"
#include "Constants.h"
#include "Food.h"
 
 
extern Food food;
 
Snake::Snake()
{
    size = 2;
    direction = GLUT_KEY_UP;
    block[0].x = width / 2 ;
    block[0].y = height / 2;
    block[1].x = width / 2;
    block[1].y = height / 2 - size_cell;
}
 
void Snake::draw()
{
    glColor3f(0.0, 1.0, 0.0);
 
    for (int i = 0; i < size; i++)
        glRectd(block[i].x + 1, block[i].y + 1, block[i].x + size_cell, block[i].y + size_cell);
}
 
void Snake::setDirection(int dir)
{
    if ((direction == GLUT_KEY_UP) && (dir != GLUT_KEY_DOWN))
        direction = dir;
    else if ((direction == GLUT_KEY_RIGHT) && (dir != GLUT_KEY_LEFT))
        direction = dir;
    else if ((direction == GLUT_KEY_DOWN) && (dir != GLUT_KEY_UP))
        direction = dir;
    else if ((direction == GLUT_KEY_LEFT) && (dir != GLUT_KEY_RIGHT))
        direction = dir;
}
 
void Snake::move()
{
    for (int i = size; i > 0; i--)
    {
        block[i].x = block[i - 1].x;
        block[i].y = block[i - 1].y;
    }
 
    switch (direction)
    {
    case GLUT_KEY_UP: block[0].y += size_cell; break;
    case GLUT_KEY_RIGHT: block[0].x += size_cell; break;
    case GLUT_KEY_DOWN: block[0].y -= size_cell; break;
    case GLUT_KEY_LEFT: block[0].x -= size_cell; break;
    }
    outside_window();
    cut();
    eat();
}
 
void Snake::outside_window()
{
    if (block[0].x > width - size_cell) block[0].x = 0;
    else if (block[0].x < 0) block[0].x = width - size_cell;
    else if (block[0].y > height - size_cell) block[0].y = 0;
    else if (block[0].y < 0) block[0].y = height - size_cell;
}
 
int Snake::getSize()
{
    return size;
}
 
void Snake::eat()
{
    if ((block[0].x == food.x) && (block[0].y == food.y))
    {
        size++;
        food.spawn();
    }
}
 
void Snake::cut()
{
    for (int i = 3; i < size; i++)
    {
        if ((block[0].x == block[i].x) && (block[0].y == block[i].y))
            size = i;   
    }
}
C++
1
2
3
4
5
6
7
8
9
10
11
12
//Game.h
#ifndef GAME_H
#define GAME_H
 
class Game
{
public:
    void drawBoard();
};
 
 
#endif GAME_H
C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
//Game.cpp
#include <glut.h>
 
#include "Game.h"
#include "Constants.h"
 
void Game::drawBoard()
{
    glClearColor(1, 1, 1, 1);
    glClear(GL_COLOR_BUFFER_BIT);
 
    glColor3f(0.0, 0.0, 0.0);
    glBegin(GL_LINES);
    //vertical lines
    for (int i = 0; i < width; i += size_cell)
    {
        glVertex2d(i, 0);
        glVertex2d(i, height);
    }
    //horizontal lines
    for (int i = 0; i < height; i += size_cell)
    {
        glVertex2d(0, i);
        glVertex2d(width, i);
    }
    glEnd();
}
C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
//main.cpp
#include <glut.h>
 
#include "Constants.h"
#include "Game.h"
#include "Snake.h"
#include "Food.h"
 
Game game;
Snake snake;
Food food;
 
void Draw()
{
    game.drawBoard();
    snake.draw();
    food.draw();
    glFlush();
}
 
void timer(int value)
{
    Draw();
    snake.move();   
 
    glutTimerFunc(300, timer, 0);
}
 
void keyboard(int key, int a, int b)
{
    switch (key)
    {
    case GLUT_KEY_UP: snake.setDirection(GLUT_KEY_UP); break;
    case GLUT_KEY_RIGHT: snake.setDirection(GLUT_KEY_RIGHT); break;
    case GLUT_KEY_DOWN: snake.setDirection(GLUT_KEY_DOWN); break;
    case GLUT_KEY_LEFT: snake.setDirection(GLUT_KEY_LEFT); break;
    }
}
 
int main(int argc, char** argv)
{
 
    food.spawn();
    glutInit(&argc, argv);
    glutInitWindowSize(width, height);
    glutInitWindowPosition(300, 100);
    glutInitDisplayMode(GLUT_RGB | GLUT_SINGLE);
    glutCreateWindow("Snake");
    glMatrixMode(GL_PROJECTION);
    glLoadIdentity();
    glOrtho(0, width, 0, height, -1, 1);
 
    glutDisplayFunc(Draw);
    glutTimerFunc(300, timer, 0);
    glutSpecialFunc(keyboard);
 
    glutMainLoop();
 
 
}
После регистрации реклама в сообщениях будет скрыта и будут доступны все возможности форума.
John Prick
754 / 687 / 123
Регистрация: 27.07.2012
Сообщений: 1,974
Завершенные тесты: 3
15.08.2014, 16:46     Оцените код "Змейки" #2
В классах данные-члены стоит помещать в private секцию, это одно из главных правил ООП. В твоём примере это не критично, но на будущее.
C++
1
2
3
4
5
6
7
8
9
10
11
class Food
{
public:
    void spawn();
    void draw();
    int X() { return x; }
    int Y() { return y; }
private:
    int x;
    int y;
};
Не проще ли сделать так?
C++
1
2
3
4
void keyboard(int key, int a, int b)
{
    snake.setDirection(key);
}
Aspirin
29 / 6 / 0
Регистрация: 05.10.2012
Сообщений: 117
15.08.2014, 17:05  [ТС]     Оцените код "Змейки" #3
А в классе Snake массив
C++
1
coordinates block[width*height];
тоже лучше поместить в private и определить методы получения данных из этого массива?
И ещё, нормально ли такое определение объектов, как у меня, с использованием extern?
John Prick
754 / 687 / 123
Регистрация: 27.07.2012
Сообщений: 1,974
Завершенные тесты: 3
15.08.2014, 17:10     Оцените код "Змейки" #4
Цитата Сообщение от Aspirin Посмотреть сообщение
тоже лучше поместить в private и определить методы получения данных из этого массива?
В общем случае, да.
Цитата Сообщение от Aspirin Посмотреть сообщение
И ещё, нормально ли такое определение объектов, как у меня, с использованием extern?
В принципе нормально для небольших программ. А в принципе глобальные переменные лучше не использовать.
Aspirin
29 / 6 / 0
Регистрация: 05.10.2012
Сообщений: 117
15.08.2014, 17:17  [ТС]     Оцените код "Змейки" #5
Цитата Сообщение от John Prick Посмотреть сообщение
В принципе нормально для небольших программ. А в принципе глобальные переменные лучше не использовать.
А как, например, можно было сделать, в классе Food доступ к массиву змейки без определения здесь объекта Snake.
ForEveR
Модератор
Эксперт C++
 Аватар для ForEveR
7927 / 4709 / 318
Регистрация: 24.06.2010
Сообщений: 10,524
Завершенные тесты: 3
15.08.2014, 17:19     Оцените код "Змейки" #6
Aspirin, 1 вариант - Переделать архитектуру.
2 вариант - Хранить ссылку на объект типа Snake в Food.
Aspirin
29 / 6 / 0
Регистрация: 05.10.2012
Сообщений: 117
15.08.2014, 18:34  [ТС]     Оцените код "Змейки" #7
Есть ещё такая проблема: в классе Food метод spawn
C++
1
2
3
4
5
6
7
8
9
10
11
void Food::spawn()
{
    srand(time(NULL));
    x = (rand() % n)*size_cell;
    y = (rand() % m)*size_cell;
    for (int i = 0; i < snake.getSize(); i++)
    {
        if ((snake.block[i].x == x) && (snake.block[i].y == y))
            spawn();
    }
}
Во время работы приложения, если сгенерированные координаты еды попадают на координаты тела змейки, то гененрируем новые. Приложение в этом случае валится,я не знаю почему. Когда искал в чем дело, добавил cout
C++
1
2
3
4
 if ((snake.block[i].x == x) && (snake.block[i].y == y)){
           cout << "Еда попала на змейку" << endl;
            spawn();
}
И заметил, что в этом случае приложение не валится, в случае если рекурсия была глубокая, приложение просто ждало пока выведет всю инфу в консоль через cout и продолжало работать. В чем может быть причина?
Voivoid
 Аватар для Voivoid
580 / 256 / 12
Регистрация: 31.03.2013
Сообщений: 1,283
15.08.2014, 18:54     Оцените код "Змейки" #8
Цитата Сообщение от Aspirin Посмотреть сообщение
А как, например, можно было сделать, в классе Food доступ к массиву змейки без определения здесь объекта Snake.
Очевидно, что еде нет никакой нужды знать о змее. Создавать еду должна игра, именно она должна знать о всех игровых объектах и естественно, что именно игра должна подбирать подходящее расположение для еды

Добавлено через 2 минуты
Цитата Сообщение от Aspirin Посмотреть сообщение
Приложение в этом случае валится,я не знаю почему
Ну, вероятно происходит переполнение стека из-за того, что генерируются одинаковые координаты из-за того, что текущее время не успевает измениться

Добавлено через 7 минут
В целом код на 3- ( честно говоря лениво все расписывать ), но по сравнению с кодом, который периодически выкладывают на этом форуме, твой выглядит более-менее неплохо
kravam
быдлокодер
 Аватар для kravam
1512 / 872 / 44
Регистрация: 04.06.2008
Сообщений: 5,265
15.08.2014, 22:01     Оцените код "Змейки" #9
Может, я чего не догоняю, но объясни мне этот участок кода:

C++
1
2
3
4
5
6
7
8
void Snake::cut()
{
    for (int i = 3; i < size; i++)
    {
        if ((block[0].x == block[i].x) && (block[0].y == block[i].y))
            size = i;   
    }
}
Итак, вошли в цикл, проходим итерацию за итерацией и однажды это условие выполнится

C++
1
((block[0].x == block[i].x) && (block[0].y == block[i].y))
(То есть все условия рано или поздно выполняются, на то они и условия).

После этого получаем, что size становится равным i; а поскольку i инкременируется (и на следующей итерации становится БОЛЬШЕ size на 1),а size не меняется,

for (int i = 3; i < size; i++)

Мы попадаем в вечный цикл... Ну, до тех пор. пока не случится переполнение i. Я не прав?
John Prick
754 / 687 / 123
Регистрация: 27.07.2012
Сообщений: 1,974
Завершенные тесты: 3
15.08.2014, 22:12     Оцените код "Змейки" #10
Цитата Сообщение от kravam Посмотреть сообщение
Мы попадаем в вечный цикл... Ну, до тех пор. пока не случится переполнение i. Я не прав?
Нет. size приравнивается i, потом i увеличивается на 1, а затем уже в начале следующей итерации цикла проверяется условие i < size. Короче, как только выполнится size = i, цикл сразу закончится.

Но ты прав в другом - цикл организован через ж...

Добавлено через 4 минуты
Цитата Сообщение от Aspirin Посмотреть сообщение
А как, например, можно было сделать, в классе Food доступ к массиву змейки без определения здесь объекта Snake.
Всё существенно переработать. Змейка ничего не знает о еде, еда ничего не знает о змейке. Слопала змейка еду или нет, должна знать сама игра: когда координаты начала змейки и еды сравняются. Как-то так.

Кстати, подумай на досуге, что в твоей программе пришлось бы поменять, если бы объектов еды потребовалось бы создать больше 1?
kravam
быдлокодер
 Аватар для kravam
1512 / 872 / 44
Регистрация: 04.06.2008
Сообщений: 5,265
15.08.2014, 22:15     Оцените код "Змейки" #11
виноват
Aspirin
29 / 6 / 0
Регистрация: 05.10.2012
Сообщений: 117
15.08.2014, 22:44  [ТС]     Оцените код "Змейки" #12
Цитата Сообщение от kravam Посмотреть сообщение
Может, я чего не догоняю, но объясни мне этот участок кода:

...

Мы попадаем в вечный цикл... Ну, до тех пор. пока не случится переполнение i. Я не прав?
И правда, я упустил этот момент, я хотел поставить там break после того, как выполнится условие.
Хотя не вижу не чего плохого в такой ситуации, т.к. по сказанному выше все работает)
kravam
быдлокодер
 Аватар для kravam
1512 / 872 / 44
Регистрация: 04.06.2008
Сообщений: 5,265
15.08.2014, 23:13     Оцените код "Змейки" #13
Цитата Сообщение от Aspirin Посмотреть сообщение
И правда, я упустил этот момент, я хотел поставить там break после того, как выполнится условие.
Хотя не вижу не чего плохого в такой ситуации)
В вечный цикл мы не попадём. как выяснилось. при выполнении условия size сравняется с i, потом выполнится i++, после чего i станет больше size и произойдёт выход из цикла. Если это то, что ты хотел, тогда самое оно.

++++++++++++++++++++++++++++++++++++++++++++++++++++++

Теперь по рациональности. Повременим пока с break. Смотри, насколько я понял, цель этого кода, в КАЖДОЙ итерации при выполнении условия увеличивать i на 1. Действительно, ты проверяешь одно условие,
C++
1
i<size
следом второе
C++
1
!((block[0].x == block[i].x) && (block[0].y == block[i].y)))
(обрати внимание на восклицательный знак, у тебя его нет, а в моём объяснении он необходим!)

и при истинности каждого из этих условий инкременируешь i. Но уже за нас всё придумано. Два условия нужно объединить в одно, вот так:

C++
1
(i<size)&&(!((block[0].x == block[i].x) && (block[0].y == block[i].y)))
И проверять их в цикле вот так:
C++
1
2
3
    i= 3;
    while ((i<size)&&(!((block[0].x == block[i].x) && (block[0].y == block[i].y))))
       i++;
++++++++++++++++++++++++++++++++++++++++++++++

Наконец, последнее, по size. Если цель size=i, выйти из цикла, тогда действительно либо break, либо, если использовать мой вариант ВООБЩЕ обходимся без этого приравнивания. Если действительно есть необходимость приравнять size к i ПОМИМО выхода из цикла, тогда пишем так:

C++
1
2
3
4
    i= 3;
    while ((i<size)&&(!((block[0].x == block[i].x) && (block[0].y == block[i].y))))
       i++;
    size= 1;
Это значит, что как только условие, которое
C++
1
((block[0].x == block[i].x) && (block[0].y == block[i].y))
выполнится, произойдёт выход из цикла и только ПОСЛЕ этого size приравняется к i, (если в этом будет необходимость, а если нет убери эту строку и всё.)

Добавлено через 5 минут
............
John Prick
754 / 687 / 123
Регистрация: 27.07.2012
Сообщений: 1,974
Завершенные тесты: 3
15.08.2014, 23:17     Оцените код "Змейки" #14
C++
1
2
3
4
5
6
7
8
void Snake::cut()
{
    for (int i = 3; i < size; i++)
    {
        if ((block[0].x == block[i].x) && (block[0].y == block[i].y))
            size = i;   
    }
}
Да вообще мутный кусок кода. Что будет, если в массиве block будет меньше 4х элементов?
Aspirin
29 / 6 / 0
Регистрация: 05.10.2012
Сообщений: 117
15.08.2014, 23:59  [ТС]     Оцените код "Змейки" #15
Цитата Сообщение от kravam Посмотреть сообщение
Наконец, последнее, по size. Если цель size=i, выйти из цикла...
Цель - при выполнении условия изменить size на номер текущей итерации.
"Физический смысл":
Этот метод - разрезание змейки. В цикле мы проходим по телу змейку(начиная от 3, т.к змейка длинной 4 и меньше не может разрезать себя) и проверяем попала ли "голова" змейки на её тело, если условие выполняется, изменяем длину змейки на номер текущей итерации(длина змейки становится равна номеру блока в котором произошел разрез) и выходим из цикла.

Цитата Сообщение от John Prick Посмотреть сообщение
Да вообще мутный кусок кода. Что будет, если в массиве block будет меньше 4х элементов?
Если будет меньше, то мы просто в цикл не войдем.
John Prick
754 / 687 / 123
Регистрация: 27.07.2012
Сообщений: 1,974
Завершенные тесты: 3
16.08.2014, 00:00     Оцените код "Змейки" #16
Цитата Сообщение от Aspirin Посмотреть сообщение
Если будет меньше, то мы просто в цикл не войдем.
По коду это не очевидно.
MoreAnswers
Эксперт
37091 / 29110 / 5898
Регистрация: 17.06.2006
Сообщений: 43,301
16.08.2014, 00:04     Оцените код "Змейки"
Еще ссылки по теме:

C++ Кто поможет вкратце описать код "Определитель матрицы"?
C++ Переменные "емкость", "Галлон", "Бензин"
Классы "Фигура", "Прямоугольник", "Круг" C++

Искать еще темы с ответами

Или воспользуйтесь поиском по форуму:
kravam
быдлокодер
 Аватар для kravam
1512 / 872 / 44
Регистрация: 04.06.2008
Сообщений: 5,265
16.08.2014, 00:04     Оцените код "Змейки" #17
Цитата Сообщение от Aspirin Посмотреть сообщение
Цель - при выполнении условия изменить size на номер текущей итерации.
значит так:
C++
1
2
3
4
i= 3;
while ((i<size)&&(!((block[0].x == block[i].x) && (block[0].y == block[i].y))))
    i++;
size= i;
Yandex
Объявления
16.08.2014, 00:04     Оцените код "Змейки"
Ответ Создать тему
Опции темы

Текущее время: 10:48. Часовой пояс GMT +3.
КиберФорум - форум программистов, компьютерный форум, программирование
Powered by vBulletin® Version 3.8.9
Copyright ©2000 - 2016, vBulletin Solutions, Inc.
Рейтинг@Mail.ru