Оцените код игры, плиз.

Slonopotam

24 января 2015

Привет. Это моя первая полноценная законченная программа — результат выполнения уроков сайта и самообразования. Чувства переполняют, но жена мою игру нифига не оценила :*(
Поэтому выкладываю сюда. Идея конечно же не моя — с давних пор валялся в компе англоязычный экзешник неизвестного происхождения. Написал русскоязычную версию по его мотивам. Хотелось бы услышать конструктивную критику.

#include <clocale>
#include <iostream>
#include <cstdlib>
#include <ctime>

using namespace std;

void comp_move();
void user_move(int, int, int);

void user_move(int comp_num, int maximal, int life) // Функция ввода чисел игроком и сравнения их с загаданным
{
    int user_num; // Число вводимое игроком

    if(life==0)
    {
        cout << "ВЫ ПРОИГРАЛИ! Было загадано число: " << comp_num << endl << endl;
        comp_move();        
    }

    cout << "Введите ваше число: ";
    cin >> user_num;

    if(user_num < 1 || user_num > maximal) // Проверка, чтобы введённое игроком число
    {                                      // не выходило за пределы заданного диапазона
        cout << "\nОШИБКА! Введите значение от 1 до " << maximal << endl;
        user_move(comp_num, maximal, life);
    }

    int alt; // Переменная для оператора switch
    user_num > comp_num ? alt=1 : user_num < comp_num ? alt=2 : alt=3; // Сравнение числа игрока с загаданным

    switch(alt) // Реакция на результат сравнения
    {
    case 1:
            life--;
            cout << "Загаданное число - меньше вашего. Осталось попыток " << life << ".\n\n";
            user_move(comp_num, maximal, life);
            break;
    case 2:
            life--;
            cout << "Загаданное число - больше вашего. Осталось попыток " << life << ".\n\n";
            user_move(comp_num, maximal, life);
            break;
    case 3:
            cout << "ВЫ УГАДАЛИ!\n\n";
            break;
    }
    comp_move();
}

void comp_move() // Функция выбора уровня сложности игры и загадывания числа компьютером
{
    cout << "Выбор уровня сложности." << endl
         << "1. Лёгкий  (1-15)" << endl
         << "2. Средний (1-30)" << endl
         << "3. Тяжёлый (1-50)" << endl
         << "Введите номер выбранного вами уровня: ";

    int mode; // Уровень сложности
    cin >> mode; // Ввод уровня сложности игроком

    int maximal; // Ограничитель максимально числа

    switch(mode) // Установка ограничителя maximal, в зависимости от уровня сложности игры
    {
        case 1:
                maximal = 15;
                break;
        case 2:
                maximal = 30;
                break;
        case 3:
                maximal = 50;
                break;
        default:
                cout << "\nОшибка! Допустимые значения: 1, 2, 3.\n\n";
                comp_move();
        break;
    }

    int comp_num; // Число загадываемое компьютером
    srand(time(0));
    comp_num = 1+rand()%maximal;
    int life = 5; // Количество попыток

    user_move(comp_num, maximal, life);
}

int main()
{
    setlocale(LC_CTYPE,".1251");

    cout << "              @@@@@@   Игра \"УГАДАЙ ЧИСЛО!\"   @@@@@@" << endl
         << "         Цель игры - угадать число загаданное компьютером." << endl
         << "        У вас пять попыток. После каждой неудачной попытки, " << endl
         << "                     компьютер даёт подсказку." << endl
         << "                   @@@@@@    Удачи!!!    @@@@@@" << endl << endl
         << "#######################################################################" << endl << endl;

    comp_move();
}

porshe

24 января 2015

Выскажу своё скромное мнение :)

Первое, что бросилось в глаза, это не совсем понятная организация объявлений и реализаций функций comp_move и user_move. Зачем объявлять их, и тут же реализовывать? Достаточно только реализации, если вы сначала реализуете все вспомогателнные функции, а только потом main. Хотя, в кошерном виде, структура файла, содержащего реализацию ф-ции main должа иметь следующий вид:

Включение заголовочных файлов

Объявление глобальных переменных

Объявление вспомогательных функций

Реализация main

Реализация вспомогательных функций

И, вообще, реализацию функций, как и их определние лучше выносить в отдельные файлы, иначе, при добавлении пары-тройки функций вы получите плохочитаемый код.

Так же, если я не ошибаюсь, вызовы функций построены с ошибкой. Дело в том, что у вас функция comp__move вызывает user_move, а user_move вызывает comp_move. Таким образом, программый стек будет забиваться адресами возврата из функций, и, после очень долгой игры , ваша программа получит segmentation fault. Но это мелочи.

Теперь по реализациям функций.

Функция comp_move реализована, на мой взгляд, хорошо. Единственный минус — srand(time(0)) нужно перенести в main, поскольку достаточно одного вызова этой функции.

Функция user_move реализована не очень хорошо :(

Проверку выхода за пределы лучше реализовывать в цикле, дабы экономить стек и избегать спагетти на своём экране.

do
{
   Чтение и проверка
}while( введённое_значение_неверно );

Реакцию на результат сравнения тоже можно вынести в цикл, причины см. выше.

while ( life )
{
читаем результат и реагируем
}

Причём сравнение, лучше реализовывать не с помощью user_num > comp_num ? alt=1 : user_num < comp_num ? alt=2 : alt=3;, а при помощи старых добрых конструкций if. Так и код становится понятней, и память экономится (это я про переменную alt).

Slonopotam

24 января 2015

Большое спасибо за отзыв! Учту ваши замечания. Что касается

у вас функция comp__move вызывает user_move, а user_move вызывает comp_move

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

не совсем понятная организация объявлений и реализаций функций comp_move и user_move. Зачем объявлять их, и тут же реализовывать?

Если не объявлять, компилятор встречает вызов comp_move до её реализации и выдает ошибку
[Error] 'comp_move' was not declared in this scope

porshe

24 января 2015

Если не объявлять, компилятор встречает вызов comp_move

А, ну да, моя ошибка :\

Cranium

24 января 2015

Slonopotam, такое использование рекурсии при длительной игре приведёт к аварийному завершению приложения. Если хочешь проверить, сделай файл из символов '1', разделённых пробелом, размером 10-20-100Кб и подай на вход своей программе:

game.exe <data.txt

У меня твоя программа вылетала уже при размере файла 10Кб (компилировал в MSVS C++ 2013 со стандартными настройками).

Вообще, такое использование рекурсии — однозначно вред. Для организации циклов в языке имеются специальные конструкции. Это и безопаснее и быстрее работает.

selevit

24 января 2015

я это сделал специально, чтобы игра длилась, пока пользователю не надоест играть и он сам не закроет программу

Сделал бы бесконечный цикл. Это обычная практика в таких делах.

while (1) {
    // Играть
}

Slonopotam

24 января 2015

О! Вот бесконечный цикл и был моей первой идеей, как это можно сделать. Но я решил, что рекурсии будут выглядеть солиднее! :))
Буду знать. Спасибо!

Olly

27 января 2015

А Вы бы попробовали выводить строчки на экране разными цветами. Так сказать, навести красоту. :) Женщины такое любят. Может быть, и Вашей жене больше понравится.

Olly

27 января 2015

Удивительным образом Ваша программа похожа на мою собственную. Только я не встречала англоязычных версий — писала сама.
Отличия, конечно, тоже есть, но они легко поправимы. 1) В моей программе не задан уровень сложности. Всегда выбирается число от 1 до 50. 2) У меня в программе нет бесконечного цикла, она работает только один раз.

Olly

27 января 2015

Удивительным образом Ваша программа похожа на мою собственную. Только я не встречала англоязычных версий - писала сама.

Отличия, конечно, тоже есть, но они легко поправимы. 1) В моей программе не задан уровень сложности. Всегда выбирается число от 1 до 50. 2) У меня в программе нет бесконечного цикла, она работает только один раз.

Slonopotam

27 января 2015

Я думаю существует много реализаций этой программы на разных языках, учитывая её простоту. А до разноцветных строчек я еще обязательно доберусь. :)

Cranium

27 января 2015

Вариант без рекурсии с минимальными правками:

#include <clocale>
#include <iostream>
#include <cstdlib>
#include <ctime>

using namespace std;


void user_move(int comp_num, int maximal) // Функция ввода чисел игроком и сравнения их с загаданным
{
    int life = 5;       // Количество попыток
    int user_num;       // Число вводимое игроком
    bool match = false; // угадали
    bool correct;

    do {
        do {
            cout << "Введите ваше число: ";
            cin >> user_num;
            correct = user_num >= 1 && user_num <= maximal;
            if (!correct)
                cout << "\nОШИБКА! Введите значение от 1 до " << maximal << endl;
        } while (!correct);

        life--;
        if (user_num > comp_num)
            cout << "Загаданное число - меньше вашего. Осталось попыток " << life << ".\n\n";
        else if (user_num < comp_num)
            cout << "Загаданное число - больше вашего. Осталось попыток " << life << ".\n\n";
        else
            match = true;

    } while (life > 0 && !match);

    if (match)
        cout << "ВЫ УГАДАЛИ!\n\n";
    else
        cout << "ВЫ ПРОИГРАЛИ! Было загадано число: " << comp_num << endl << endl;
}

void comp_move(int& comp_num, int& maximal) // Функция выбора уровня сложности игры и загадывания числа компьютером
{
    int mode; // Уровень сложности
    bool correct;

    do {
        cout << "Выбор уровня сложности." << endl
            << "1. Лёгкий  (1-15)" << endl
            << "2. Средний (1-30)" << endl
            << "3. Тяжёлый (1-50)" << endl
            << "Введите номер выбранного вами уровня: ";

        cin >> mode; // Ввод уровня сложности игроком
        correct = mode > 0 && mode <= 3;
        if (!correct)
            cout << "\nОшибка! Допустимые значения: 1, 2, 3.\n\n";
    } while (!correct);

    switch (mode) // Установка ограничителя maximal, в зависимости от уровня сложности игры
    {
    case 1:
        maximal = 15;
        break;
    case 2:
        maximal = 30;
        break;
    case 3:
        maximal = 50;
        break;
    }

    comp_num = 1 + rand() % maximal;
}

int main()
{
    setlocale(LC_CTYPE, ".1251");

    cout << "              @@@@@@   Игра \"УГАДАЙ ЧИСЛО!\"   @@@@@@" << endl
        << "         Цель игры - угадать число загаданное компьютером." << endl
        << "        У вас пять попыток. После каждой неудачной попытки, " << endl
        << "                     компьютер даёт подсказку." << endl
        << "                   @@@@@@    Удачи!!!    @@@@@@" << endl << endl
        << "#######################################################################" << endl << endl;

    int comp_num;   // Число загадываемое компьютером
    int maximal;    // Ограничитель максимально числа

    srand(static_cast<unsigned int>(time(NULL)));

    while (true) {
        comp_move(comp_num, maximal);
        user_move(comp_num, maximal);
    }

    return 0;
}

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

Cranium

27 января 2015

Кстати, гарантированно отгадать число из интервала 1..50 за 5 ходов невозможно. Этот уровень сложности надо назвать «Вам повезёт!».

Olly

27 января 2015

Вот код моей программы. Разноцветной.
Ещё одно отличие в том, что здесь кол-во попыток не задаётся заранее, а определяется по ходу. Она короче и проще (например, здесь нет ни одной функции).

#include <iostream> 
#include <ctime> 
#include <cstdlib>
#include <windows.h>

using namespace std;

int main() 
{ 
    setlocale(0, "");
    int i=0,num,a; 
    srand(time(NULL));
    num = 1 + rand() % 50;
    SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), FOREGROUND_INTENSITY | FOREGROUND_RED);
    cout<<"Я загадал целое число от 1 до 50."<<endl;
    cout<<"Попробуй отгадать!"<<endl;
    cout<<endl;

    do
    {
        SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), FOREGROUND_INTENSITY | FOREGROUND_RED | FOREGROUND_BLUE | FOREGROUND_GREEN);
        cin>>a;
        i++;
        if (num>a) 
        {
            SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), FOREGROUND_INTENSITY | FOREGROUND_BLUE);
            cout<<"Нет, задуманное число больше."<<endl;
        }
        if (num<a) 
        {
            SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), FOREGROUND_INTENSITY | FOREGROUND_GREEN);
            cout<<"Нет, задуманное число меньше."<<endl;
        }
        cout<<endl;
    }
    while (a != num);
    cout<<endl;
    SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), FOREGROUND_INTENSITY | FOREGROUND_RED);
    cout<<"Совершенно верно! Так точно."<<endl;
    cout<<endl;
    cout<<"Поздравляю! Вы потратили количество попыток = "<<i<<" ! "<<endl;
    cout<<endl;
    system("pause>>VOID");

    return 0; 
}

Фтьiкай

29 января 2015

Olly, в статье Пишем змейку на C++ есть реализация класса для вывода цветного текста на консоль. Там правда все достаточно аскетичненько, но работает. И лицензия GPL — так что можно юзать ))

Olly

30 января 2015

Фтьiкай, спасибо!
Правда, я не спрашивала... Но интересно было посмотреть чужой опыт.
Когда-то я задавалась этим вопросом. Нашла в интернете и написала программку, в которой слово «Привет» выводится разными цветами:

#include <iostream>
#include <windows.h>

int main() 
{ 
        using namespace std;
        setlocale(0, "");

SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), FOREGROUND_INTENSITY | FOREGROUND_GREEN);
        cout<<" Привет! ";
        SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), FOREGROUND_INTENSITY | FOREGROUND_RED);
        cout<<" Привет! ";
        SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), FOREGROUND_INTENSITY | FOREGROUND_RED | FOREGROUND_BLUE | FOREGROUND_GREEN);
        cout<<" Привет! ";
        SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), FOREGROUND_INTENSITY | FOREGROUND_BLUE);
        cout<<" Привет! ";
        SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), FOREGROUND_INTENSITY | FOREGROUND_RED | FOREGROUND_GREEN);
        cout<<" Привет! ";
        SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), FOREGROUND_INTENSITY | FOREGROUND_RED | FOREGROUND_BLUE);
        cout<<" Привет! ";
        SetConsoleTextAttribute(GetStdHandle(STD_OUTPUT_HANDLE), FOREGROUND_INTENSITY | FOREGROUND_GREEN | FOREGROUND_BLUE);
        cout<<" Привет! ";

        system("pause>>VOID"); 
        return 0; 
}

А потом уже я смогла выводить строчки разными цветами.

Slonopotam

1 февраля 2015

Нарыл более простой способ менять цвет и фона и текста целиком с помощью system(). Если требуется установить значение 1 раз, напишите команду в глобальной области.

#include <iostream>
#include <conio.h>
using namespace std;

int main()
{
    system("color E1");
    cout << "hello!"  << endl;

    _getch();            // нажмите любую клавишу
    system("color AC");
    cout << "hello!"  << endl;

    _getch();            // нажмите любую клавишу
    system("color 70");
    cout << "hello!"  << endl;
}

/*
первая цифра - значение цвета фона, вторая цифра - значение цвета текста

 Расшифровка цветов:
 0 = Black 8 = Gray
 1 = Blue 9 = Light Blue
 2 = Green A = Light Green
 3 = Aqua B = Light Aqua
 4 = Red C = Light Red
 5 = Purple D = Light Purple
 6 = Yellow E = Light Yellow
 7 = White F = Bright White
 */

Olly

26 февраля 2015

Slonopotam, ты — просто мастер находить нужную информацию!
Я попробовала ввести точно такой же текст. Почему-то приветствие печатается только 2 раза вместо трёх. Зато разными цветами и на разном фоне.

Cranium

26 февраля 2015

Нарыл более простой способ менять цвет и фона и текста целиком с помощью system().

Здесь два недостатка.

Цвет изменяется для всего окна консоли.
Работа через system() очень медленная.

Через системные вызовы — оно более гибко и правильно.

Или пользовать ncurses/PDcurses, если нужна переносимость.

Olly,

Почему-то приветствие печатается только 2 раза вместо трёх.

Печатается 3 раза. Только последнюю выдачу ты не успеваешь заметить, потому что окно консоли закрывается. Поставь в конце ещё один _getch() и будет тебе счастье. Или запускай в Visual Studio через Ctrl-F5.

Olly

27 февраля 2015

Cranium, поставила в конце _getch() и получила то самое счастье! Спасибо!

Внимание! Это довольно старый топик, посты в него не попадут в новые, и их никто не увидит. Пишите пост, если хотите просто дополнить топик, а чтобы задать новый вопрос — начните новый.

Ответить

Вы можете использовать разметку markdown для оформления комментариев и постов. Используйте функцию предпросмотра для проверки корректности разметки.

Пожалуйста, оформляйте исходный код в соответствии с правилами разметки. Для того, чтобы вставить код в комментарий, скопируйте его в текстовое поле ниже, после чего выделите то, что скопировали и нажмите кнопку «код» в панели инструментов. Иначе ваш код может принять нечитаемый вид.

Либо производите оформление кода вручную, следующим образом:

``` #include <iostream> using namespace std; int main() { // ... } ```

Текст

Имя

Это поле — ловушка для роботов, оставьте его пустым

Предпросмотр сообщения

Ваше сообщение пусто.