我在connect4上做了一个c++游戏,请告诉我如何使代码更好,谢谢。
我已经问过一些人了,他们告诉我,我应该使用固定的witdh ints和[]用于不能丢弃的函数,也应该使用to,除非使用不抛出的函数。
#include
#include
#include
#include
#include
class Board {
public:
enum class CellType { player1, player2, empty };
using board_type = std::array, 6>;
private:
board_type m_board{};
std::pair m_latestHit{};
std::string getCellColor(CellType type) const noexcept {
switch (type) {
case CellType::player1:
return "\033[1;33m O\033[0m"; // red
case CellType::player2:
return "\033[1;36m O\033[0m"; // blue
case CellType::empty:
return "\033[1;30m O\033[0m"; // black
}
}
public:
Board() {
for (uint8_t cols{0}; cols < 7; ++cols) {
for (uint8_t rows{0}; rows < 6; ++rows) {
m_board[rows][cols] = CellType::empty;
}
}
}
void reset() noexcept {
std::for_each(m_board.begin(), m_board.end(), [](auto &col) {
std::for_each(col.begin(), col.end(),
[](auto &cell) { cell = CellType::empty; });
});
}
void pruint8_t() const noexcept {
std::for_each(m_board.begin(), m_board.end(), [&](auto &col) {
std::cout << '\t';
std::for_each(col.begin(), col.end(),
[&](auto &cell) { std::cout << getCellColor(cell); });
std::cout << '\n';
});
};
void hit(uint8_t col, CellType player) noexcept {
// the first cell on the column that is not already hit
uint8_t freeCell{static_cast(getColDepth(col, m_board) - 1)};
m_board[freeCell][col] = player;
m_latestHit = {freeCell, col};
};
// returns how many cells are not empty in that column
[[nodiscard]] static constexpr uint8_t
getColDepth(uint8_t col, const board_type &board) noexcept {
uint8_t depth{};
// start loop at top of col
for (uint8_t row{0}; row < 6; ++row) {
if (board[row][col] != CellType::empty)
return row;
}
// if all are empty return 6
return 6;
};
[[nodiscard]] board_type getBoard() const { return m_board; };
class Check {
board_type board_h{};
Board::CellType player_h;
uint8_t m_row{};
uint8_t m_col{};
public:
Check(Board::CellType player, const board_type &board, uint8_t row,
uint8_t col)
: player_h{player}, board_h{board}, m_row(row), m_col{col} {};
[[nodiscard]] bool vertical() noexcept {
// if row is too low to be able to win return false
if (m_row >= 3)
return false;
for (uint8_t i = 0; i < 4; ++i) {
if (board_h[m_row + 1][m_col] != player_h)
return false;
};
return true;
};
[[nodiscard]] bool horizontal() noexcept {
// if center column check for win on both sides
if (m_col == 3) {
return left() || right();
// check only right side
} else if (m_col < 3) {
return right();
// check noly left side
} else if (m_col > 3) {
return left();
}
// default
return false;
};
[[nodiscard]] bool left() noexcept {
for (uint8_t i = 0; i < 4; ++i) {
if (board_h[m_row][m_col - i] != player_h)
return false;
}
return true;
};
[[nodiscard]] bool right() noexcept {
for (uint8_t i = 0; i < 4; ++i) {
if (board_h[m_row][m_col + i] != player_h)
return false;
};
return true;
};
[[nodiscard]] bool diagonal() noexcept {
// for every direction check if the next 4 are equal
// loop continues until a bad value is found
// if no bad value is found during the loop return true
// diagonal right down
for (uint8_t i = 0; i < 4; ++i) {
if (board_h[m_row + i][m_col + i] == player_h) {
if (i == 3)
return true;
else
continue;
} else
break;
};
// diagonal left down
for (uint8_t i = 0; i < 4; ++i) {
if (board_h[m_row + i][m_col - i] == player_h) {
if (i == 3)
return true;
else
continue;
} else
break;
};
// diagonal left up
for (uint8_t i = 0; i < 4; ++i) {
if (board_h[m_row - i][m_col - i] == player_h) {
if (i == 3)
return true;
else
continue;
} else
break;
}
// diagonal right up
for (uint8_t i = 0; i < 4; ++i) {
if (board_h[m_row - i][m_col + i] == player_h) {
if (i == 3)
return true;
else
continue;
} else
break;
}
// if no wins return false
return false;
}
};
[[nodiscard]] bool checkForWin(CellType player) noexcept {
uint8_t row{m_latestHit.first};
uint8_t col{m_latestHit.second};
Check check{player, m_board, row, col};
return (check.diagonal() || check.horizontal() || check.vertical());
};
};
class Player {
std::string m_name{};
Board::CellType m_player{};
public:
Player(std::string name = "", Board::CellType player = Board::CellType::empty)
: m_name{name}, m_player{player} {};
[[nodiscard]] Board::CellType getPlayer() const { return m_player; };
std::string getName() const { return m_name; };
};
class Game {
private:
Board board{};
Player m_p1{};
Player m_p2{};
std::array players{m_p1, m_p2};
[[nodiscard]] uint8_t getInput() noexcept {
std::cout << "Enter column:\n";
uint8_t col;
std::cin >> col;
return col;
}
public:
Game(Player p1, Player p2) : m_p1{p1}, m_p2{p2} {};
[[nodiscard]] Player play() {
uint8_t currentPlayer{};
uint8_t rounds{};
uint8_t maxRounds{42};
while (true) {
if (rounds == maxRounds)
throw "TIE!";
board.pruint8_t();
uint8_t col;
while (true) {
col = getInput();
// if column is full
if (Board::getColDepth(col, board.getBoard()) != 1)
break;
std::cout << "column is full!\n";
}
board.hit(col, players[currentPlayer].getPlayer());
if (board.checkForWin(players[currentPlayer].getPlayer())) {
board.pruint8_t();
return players[currentPlayer];
}
currentPlayer = (currentPlayer == 1 ? 0 : 1);
}
}
};
[[nodiscard]] bool playAgain() noexcept {
std::cout << "Restart game? [y/n]\n";
char x;
std::cin >> x;
return (toupper(x) == 'Y');
}
[[nodiscard]] std::string getName(uint8_t nth) noexcept {
std::cout << "Player " << nth << " name:\n";
std::string name{};
std::cin >> name;
return name;
}
int main() {
do {
try {
Player p1{getName(1), Board::CellType::player1};
Player p2{getName(2), Board::CellType::player2};
Game game{p1, p2};
Player winner{game.play()};
std::cout << "\nWinner: " << winner.getName() << '\n';
} catch (char *tie) {
std::cout << tie;
}
} while (playAgain());
};发布于 2022-05-29 00:11:58
using board_type = std::array, 6>;这里有两个文本,虽然它们可能是构造过程中的参数,或者至少是常量。
return "\033[1;33m O\033[0m"; // red我不明白这个字符串的类型,但它不是颜色。您需要使用行尾注释这一事实说明了这一点。描述这种字符串类型的常量会好得多。
std::pair m_latestHit{};像position这样的东西实际上比一对uints更具有描述性,即使它只是一个类型的冒险者。
for (uint8_t cols{0}; cols < 7; ++cols) {三点意见:
cols = 0而不是cols{0},但这可能是现在的最佳实践(为什么?)++cols会被认为更快,但对我来说,cols++只是更易读,我想现在每个C++程序员都知道++cols不会立即被执行,但我希望他们也知道这种优化是不必要的。void reset() noexcept {
std::for_each(m_board.begin(), m_board.end(), [](auto &col) {
std::for_each(col.begin(), col.end(),
[](auto &cell) { cell = CellType::empty; }); });
}
}我不明白为什么需要两个实现来将所有单元格设置为空,只需从构造函数和公共reset方法调用一个(私有)方法。
就我个人而言,我认为这两个语句的可读性不如构造函数中的两个for循环,但是嘿,也许这是现在的最佳实践。
void pruint8_t() const noexcept {真的不知道pruint8_t是什么意思。
[&](auto &cell) { std::cout << getCellColor(cell); });啊,看起来这个应该在一个特定的终端上运行,代码/注释应该反映这一点。
void hit(uint8_t col, CellType player) noexcept {从什么时候开始Connect4大受欢迎了?
getColDepth(uint8_t col, const board_type &board) noexcept {请注意,您已经有一些关于颜色的东西在那里,所以拼出Column只需要3个额外的字符,但我们避免了很多混乱。
Game(Player p1, Player p2) : m_p1{p1}, m_p2{p2} {};代码结束时,您会变得有点草率;当然,这应该是player1和player2。
uint8_t maxRounds{42};这与板上的单元格数相同,因此Board可以通过计算totalRows * totalColumns返回该值。
throw "TIE!";除非出了什么差错,否则永远不要扔东西。平局并不意味着比赛就坏了。
uint8_t col;我喜欢这样,不需要初始化,所以你不需要。
if (board.checkForWin(players[currentPlayer].getPlayer())) {董事会已经成为一个演员,而不仅仅是一个数据类。这不一定是坏的,但也许这是值得考虑的事情。
我喜欢的事实是,你只检查了最新的一件,你有办法避免太多的支票。我不认为从性能上讲这很重要--毕竟这需要微秒(如果不是纳秒的话)--但最好的编程实践是只执行严格必要的多个操作。
但是,我非常想知道,如果您将一段放置在横线或对角线的中间中,会发生什么。在您的代码中,检查似乎不存在。我还认为有4个检查应该足够(水平,垂直,和两个对角线)。先从一边找到起点,然后从起始部分移到另一边,数一数。
什么是“支票”课?类通常不使用动词,而是使用名词。如果您有一个可以执行检查的类,那么它可能应该被称为Checker。
我认为Connect4Checker.check(Board board, Position position)会好得多(球员可以直接从棋盘上的位置恢复,毕竟应该有一块)。
我会使用不同的坐标系,左下角是(0,0)。这样,董事会就可以不受限制地扩大到任何规模,也不需要改变你编程的规则。
如果您有一个Position类,那么您也可以定义一个北、西、东和南。有一件好事是,您可以执行neighbor = position.west(1),这可以极大地简化代码的读取,以检查是否存在connect4。这是我下象棋时做的。它还可以有一个print语句,它可以极大地帮助调试。
总之,您有合理的标识符,代码当然不会对我造成不良的格式化。空白是合理的,线的大小也一样。很高兴阅读代码,真的,一点也不坏。
https://codereview.stackexchange.com/questions/276917
复制相似问题