Как новичок в C ++, я практикую свои навыки C ++, реализуя игры в ОСНОВНЫЕ компьютерные игры а вот моя версия на C ++ для Боевой.
Предложения, которые я хочу
Любые предложения приветствуются!
Текущий код проекта с моего рабочего места, полный стилей C в C ++ (что не очень хорошо), я хочу, чтобы мой код был в стиле C ++ и больше напоминал C ++ 11 или современный стиль C ++. И я был программистом на Python, я до сих пор плохо разбираюсь в таких вещах в C ++, как static, pointer и так далее.
О моем коде
В моей реализации Battle я жестко кодирую часть игровой карты, я не очень хорошо понимаю этот XD. Так что сначала пропустим, это моя TODO
void InitMap()
{
// TODO: random map algorithm
m_gameMap = {
{0,0,0,2,2,6},
{0,4,4,4,6,0},
{5,0,0,6,0,0},
{5,0,6,0,0,3},
{5,1,0,0,0,3},
{5,0,1,0,0,3},
};
}
Игра проста, пользователь вводит координату удара, а затем сообщает пользователю результат.
Когда корабль падает (все координаты этого корабля заданы), прямо сейчас расскажите пользователю всю ситуацию в игре.
if (iAttack > 0) {
std::cout << "A Direction Hit On Ship Number " << iAttack << std::endl;
if (!game.IsShipStillSurvive(iAttack)) {
std::cout << "And you sunk it. Hurrah for the good guys." << std::endl;
std::cout << "So far the bad gut have lost" << std::endl;
game.ShipLost(iDestroyerLost, iCruisersLost, iAircraftLost);
std::cout << iDestroyerLost << " Destroyer(s), " << iCruisersLost;
std::cout << " Cruiser(s), And " << iAircraftLost << " Aircraft Carrier(s)." << std::endl;
}
} else {
std::cout << "Splash!" << std::endl;
}
Код
#include <iostream>
#include <vector>
#include <map>
#include <string>
#include <sstream>
typedef enum {
SHIP_TYPE_DESTROYER,
SHIP_TYPE_CRUISERS,
SHIP_TYPE_AIRCRAFT,
} ShipType;
typedef unsigned int ShipNumber ;
struct Coordinate {
unsigned int x;
unsigned int y;
};
class Game
{
public:
Game()
{
m_shipLiveInfo.clear();
for (auto iter = m_shipNumberInfo.begin(); iter != m_shipNumberInfo.end(); ++iter) {
unsigned int iLive = 0;
auto fleetIter = m_shipFleetInfo.find(iter->second);
if (fleetIter != m_shipFleetInfo.end()) {
iLive = fleetIter->second;
}
m_shipLiveInfo.insert(std::pair<ShipNumber, unsigned int>(iter->first, iLive));
}
InitMap();
}
virtual ~Game() {}
void Attack(const Coordinate& coordinate, ShipNumber& iAttackShip)
{
iAttackShip = 0;
if (coordinate.x <= m_iMapWidth && coordinate.y <= m_iMapHeight && coordinate.x > 0 && coordinate.y > 0) {
iAttackShip = m_gameMap[coordinate.x - 1][coordinate.y - 1];
std::cout << "Attack " << iAttackShip << std::endl;
m_gameMap[coordinate.x - 1][coordinate.y - 1] = 0;
auto iter = m_shipLiveInfo.find(iAttackShip);
if (iter != m_shipLiveInfo.end()) {
--(iter->second);
}
}
}
bool IsShipStillSurvive(ShipNumber& iShip)
{
std::map<ShipNumber, unsigned int>::iterator iter = m_shipLiveInfo.find(iShip);
if (iter == m_shipLiveInfo.end()) {
return true;
}
return (iter->second > 0);
}
bool IsGameOver()
{
for (auto iter = m_shipLiveInfo.begin(); iter != m_shipLiveInfo.end(); ++iter) {
if (iter->second > 0) {
return false;
}
}
return true;
}
void ShipLost(unsigned int& iDestroyer, unsigned int& iCruisers, unsigned int& iAircraft)
{
for (auto iter = m_shipLiveInfo.begin(); iter != m_shipLiveInfo.end(); ++iter) {
auto shipIter = m_shipNumberInfo.find(iter->first);
if (shipIter == m_shipNumberInfo.end()) {
continue;
}
ShipType shipType = shipIter->second;
if (iter->second == 0) {
switch (shipType) {
case SHIP_TYPE_DESTROYER:
++iDestroyer;
break;
case SHIP_TYPE_CRUISERS:
++iCruisers;
break;
case SHIP_TYPE_AIRCRAFT:
++iAircraft;
break;
}
}
}
}
static const unsigned int m_iMapWidth = 6;
static const unsigned int m_iMapHeight = 6;
private:
std::vector<std::vector<ShipNumber>> m_gameMap;
std::map<ShipNumber, unsigned int> m_shipLiveInfo;
static const std::map<ShipType, unsigned int> m_shipFleetInfo;
static const std::map<unsigned int, ShipType> m_shipNumberInfo;
void InitMap()
{
// TODO: random map algorithm
m_gameMap = {
{0,0,0,2,2,6},
{0,4,4,4,6,0},
{5,0,0,6,0,0},
{5,0,6,0,0,3},
{5,1,0,0,0,3},
{5,0,1,0,0,3},
};
}
};
const std::map<ShipNumber, ShipType> Game::m_shipNumberInfo = {
{1, SHIP_TYPE_DESTROYER},
{2, SHIP_TYPE_DESTROYER},
{3, SHIP_TYPE_CRUISERS},
{4, SHIP_TYPE_CRUISERS},
{5, SHIP_TYPE_AIRCRAFT},
{6, SHIP_TYPE_AIRCRAFT},
};
const std::map<ShipType, unsigned int> Game::m_shipFleetInfo = {
{SHIP_TYPE_DESTROYER, 2},
{SHIP_TYPE_CRUISERS, 3},
{SHIP_TYPE_AIRCRAFT, 4}
};
int main(int argc, char** argv)
{
Game game = Game();
Coordinate cor;
std::string strPosition;
std::vector<unsigned int> positionNums;
ShipNumber iAttack;
bool isFirstStart = true;
unsigned int iDestroyerLost = 0;
unsigned int iCruisersLost = 0;
unsigned int iAircraftLost = 0;
while (!game.IsGameOver()) {
iDestroyerLost = 0;
iCruisersLost = 0;
iAircraftLost = 0;
if (isFirstStart) {
std::cout << "Start Game" << std::endl;
isFirstStart = false;
} else {
std::cout << "Try Again" << std::endl;
}
positionNums.clear();
std::cin >> strPosition;
std::stringstream ss(strPosition);
for (unsigned int i; ss>>i;) {
positionNums.push_back(i);
if (ss.peek() == ',') {
ss.ignore();
}
}
if (positionNums.size() < 2) {
std::cout << "InValid Input" << std::endl;
continue;
}
cor.x = positionNums[0];
cor.y = positionNums[1];
if (cor.x > Game::m_iMapWidth || cor.x == 0 || cor.y > Game::m_iMapHeight || cor.y == 0) {
std::cout << "InValid Input" << std::endl;
continue;
}
game.Attack(cor, iAttack);
if (iAttack > 0) {
std::cout << "A Direction Hit On Ship Number " << iAttack << std::endl;
if (!game.IsShipStillSurvive(iAttack)) {
std::cout << "And you sunk it. Hurrah for the good guys." << std::endl;
std::cout << "So far the bad gut have lost" << std::endl;
game.ShipLost(iDestroyerLost, iCruisersLost, iAircraftLost);
std::cout << iDestroyerLost << " Destroyer(s), " << iCruisersLost;
std::cout << " Cruiser(s), And " << iAircraftLost << " Aircraft Carrier(s)." << std::endl;
}
} else {
std::cout << "Splash!" << std::endl;
}
}
return 0;
}
2 ответа
Код неплохой, но я вижу некоторые вещи, которые могут помочь вам его улучшить.
Проверить написание
В строках, отображаемых для пользователя, есть несколько опечаток и своеобразное использование заглавных букв. Например, «кишка», вероятно, должна быть «парни», «Недействительный» должен быть «Недействительным», а «Направление попадания» должно быть «прямым попаданием». Небольшие дополнительные усилия помогут пользователю.
Пропустить автоконструируемые функции
В настоящий момент в коде есть такая строка:
virtual ~Game() {}
Разумно сделать деструктор виртуальным, если вы ожидаете, что он будет производным от него, но здесь это не так, поэтому я бы рекомендовал просто опустить его и позволить компилятору сгенерировать деструктор по умолчанию.
Использовать const где это возможно
Во многих случаях, например, IsGameOver(), код не должен и не должен изменять базовый Game объект, и поэтому он должен быть объявлен const:
bool IsGameOver() const;
Предпочитать std::array к std::vector если размеры фиксированы во время компиляции
Поскольку мы знаем размеры во время компиляции, везде, где вы использовали std::vector, Я бы предложил использовать std::array вместо. Для этого кода это, вероятно, не будет иметь большого значения, но полезно выработать привычку использовать наиболее экономичный тип данных, который является практичным.
Не использовать std::endl если тебе это действительно не нужно
Разница между std::endl и 'n' в том, что 'n' просто испускает символ новой строки, а std::endl фактически сбрасывает поток. Это может занять много времени в программе с большим количеством операций ввода-вывода и на самом деле редко требуется. Лучше всего Только использовать std::endl когда у вас есть веская причина для очистки потока, а это не очень часто требуется для простых программ, таких как эта. Избегайте привычки использовать std::endl когда 'n' будет делать, принесет дивиденды в будущем, поскольку вы будете писать более сложные программы с большим количеством операций ввода-вывода и в которых необходимо максимизировать производительность.
Переосмыслите использование объектов
В Game класс — хорошая идея, но информация о кораблях разбросана по разным местам. Например, m_shipNumberInfo имеет карту номера корабля к типу корабля, а затем m_shipFleetInfo имеет карту типа корабля к длине корабля. Оба ссылаются на enum ShipType. Для этого я бы предложил вместо этого использовать объекты. Начать можно с этого:
class Ship {
public:
enum ShipType { DESTROYER=2, CRUISER=3, AIRCRAFT=4 };
Ship(ShipType t) : myType{t}, remaining{t} {}
unsigned hit() { return remaining ? --remaining : 0; }
// return true if ship is still alive
bool operator()() const { return remaining; }
// data members
ShipType myType;
unsigned remaining;
};
Теперь вся информация о корабле собрана в одном месте. Затем мы можем сделать этот класс private в пределах Game и отслеживайте их с помощью инициализации по умолчанию, например:
std::array<Ship, 6> ships = {
Ship::ShipType::DESTROYER,
Ship::ShipType::DESTROYER,
Ship::ShipType::CRUISER,
Ship::ShipType::CRUISER,
Ship::ShipType::CARRIER,
Ship::ShipType::CARRIER,
};
Теперь многое стало намного проще. Например:
bool IsShipStillSurvive(ShipNumber& iShip) const
{
return ships.at(iShip - 1)();
}
bool IsGameOver() const
{
return std::none_of(ships.begin(), ships.end(), [](const Ship& s){return s();});
}
Подумайте о пользователе
В игре очень мало инструкций или отзывов, что затрудняет игру. Я бы посоветовал добавить несколько инструкций, а также предоставить пользователю карту на каждом повороте, чтобы показать места, где уже были совершены атаки, и какие из них привели к ударам.
Рассмотрим дополнительные объекты
В добавок к Ship упомянутый выше объект, также было бы неплохо отделить Board объект из Game и использовать Модель-Представление-Контейнер подход. Так же Coordinate может быть публичным объектом внутри Game а ввод и проверка могут быть функцией-членом этого класса, а не в main.
Используйте возможности C ++ 20
Одна удобная функция C ++ 20 — возможность возвращать std::tuple. Итак, вместо этого:
game.ShipLost(iDestroyerLost, iCruisersLost, iAircraftLost);
Вместо этого мы можем написать это:
auto [iDestroyerLost, iCruisersLost, iAircraftLost] = game.ShipLost();
Соответствующую функцию можно упростить, используя std::count_if:
std::tuple<unsigned, unsigned, unsigned > ShipLost()
{
unsigned iDestroyer = std::count_if(ships.begin(), ships.end(), [](const Ship& s){
return s.myType == Ship::ShipType::DESTROYER && !s(); });
unsigned iCruisers = std::count_if(ships.begin(), ships.end(), [](const Ship& s){
return s.myType == Ship::ShipType::CRUISER && !s(); });
unsigned iAircraft = std::count_if(ships.begin(), ships.end(), [](const Ship& s){
return s.myType == Ship::ShipType::CARRIER && !s(); });
return std::make_tuple(iDestroyer, iCruisers, iAircraft);
}
Во-первых, ваш «TODO» для заполнения карты хорош. Это пример нисходящей декомпозиции, и неплохо было бы заблокировать ваш код и начать с заглушки.
typedef enum {
SHIP_TYPE_DESTROYER,
SHIP_TYPE_CRUISERS,
SHIP_TYPE_AIRCRAFT,
} ShipType;
typedef unsigned int ShipNumber ;
typedef enum? Это не C. Write enum ShipType { ⋯ }; или даже enum class в зависимости от ваших потребностей. Вам не нужно typedef что такое «теги» в C для получения имен типов; они уже являются именами типов в C ++.
Хорошо давать вашим типам символические имена, даже если они не являются «сильными типами», но не используйте typedef ключевое слово больше, вообще. Напишите это как:
using ShipNumber = unsigned int;
или еще лучше, используйте <stdint> такие как uint32_t.
Зачем ты делаешь свой деструктор virtual? У вас есть нет (другие) виртуальные функции в классе, и вы никоим образом не используете полиморфизм. Вы используете сам класс, а не производные классы.
Game game = Game();
Только:
Game game;
это то, что вам нужно. Ты понимаешь это Game создается в стеке, а не в куче? Интересно, думаете ли вы, что это похоже на Java, потому что другие переменные не инициализируются таким образом.
Используйте целочисленные типы со знаком, а не unsigned, если вы не выполняете манипуляции с битами, полагайтесь на переполнение, или очень нужно еще один бит диапазона.
Coordinate cor;
std::string strPosition;
std::vector<unsigned int> positionNums;
Они определены как часть группы в начале main. Но вам не нужно, чтобы они сохраняли свое значение при итерациях цикла; на самом деле, вы звоните positionNums.clear() в верхней части петли. Объявить переменные там, где они вам нужны в первую очередь.
Абстрагируйте ввод / взаимодействие пользователей
Вместо смешивания cin >> вещи в огромном цикле, который делает все,
⭐ писать функции, чтобы делать одна вещь как единый уровень абстракции дальше вниз.
Ваш игровой цикл должен выглядеть как набросок, с именованными функциями, выражающими шаги высокого уровня, а не кусками кода. делает эти шаги.
Что касается ввода, в частности, используя cin>> неприятен тем, что противоречит лучшим методам определения переменных там, где они вам нужны, их инициализации и, надеюсь, их создания const. А затем вам нужно проанализировать введенные данные и исправить ошибки на всех более «глубоких» уровнях детализации, чем вам нужно. Основной цикл должен указывать просто:
const Coordinate cor = get_play();
Весь этот кусок кода:
positionNums.clear();
std::cin >> strPosition;
std::stringstream ss(strPosition);
for (unsigned int i; ss>>i;) {
positionNums.push_back(i);
if (ss.peek() == ',') {
ss.ignore();
}
}
if (positionNums.size() < 2) {
std::cout << "InValid Input" << std::endl;
continue;
}
cor.x = positionNums[0];
cor.y = positionNums[1];
это деталь, которая не принадлежит здесь в основном цикле.
В любом случае полезно абстрагироваться от ввода / взаимодействия как отдельного предмета. В порте BASIC вы можете использовать ввод терминала, как здесь, но вы можете позже заменить его каким-либо графическим интерфейсом или элементами управления игрой, просто изменив реализацию этих четко определенных функций и вообще не влияя на основную игровую логику. .
void Attack(const Coordinate& coordinate, ShipNumber& iAttackShip)
Я предполагаю, что второй должен быть «выходным» параметром, и эта функция предоставляет эту информацию вызывающему.
Строго избегайте «выходных» параметров.
Возврат вещей с использованием значений, возвращаемых функцией.
if (coordinate.x <= m_iMapWidth && coordinate.y <= m_iMapHeight && coordinate.x > 0 && coordinate.y > 0) {
// the entire meat of the function
}
Напишите «в левое поле». Не открывайте все более и более глубокие уровни вложенности. У вас есть предварительное условие. Поскольку это глобальное ограничение (вся карта), оно должно быть уже проверено как часть функции ввода, и эта функция может предполагать, что ей передано допустимое значение coordinate.
Но, чтобы проиллюстрировать, предварительные условия должны возвращать или бросать в качестве прелюдии к реальному телу функции. Вместо того, чтобы открывать еще один уровень вложенности:
if (!mapbounds.contains(coordinate)) return 0;
И обратите внимание, что проверка того, находится ли точка внутри прямоугольника, — это обычная операция многократного использования. Вы делаете его более сложным и менее «высокоуровневым», сохраняя размер карты в двух отдельных переменных, а не в другой паре координат.
[coordinate.x - 1][coordinate.y - 1]
Вы постоянно корректируете индексы при доступе к своим игровым данным. Используйте значения внутренних координат, которые соответствовать представительство правления. То есть с нуля. Регулировка должна выполняться как часть пользовательский ввод / вывод. Фактически, на реальной доске линкора вы используете буквы для одной из осей, так что на самом деле это будет A через K или какой бы большой ни была доска. Таким образом, отображение пользовательского ввода-вывода является более общим, чем просто вычитание 1; это может быть совсем другое представление. Одна ось может вычитать 1 из введенного числа, другая отображает букву на число. Точно так же отображение координат — это проблема ввода-вывода пользователя, и функции вывода должны принимать то же самое. coordinate это используется структурой данных.