文章/答案/技术大牛

发布

社区首页 >问答首页 >自定义纸牌游戏

问自定义纸牌游戏
EN

Code Review用户

提问于 2016-03-23 23:59:20

回答 1查看 2.3K关注 0票数 6

我的任务是创建一个程序来模拟一个简单的纸牌游戏。给出一个输入文件，其中包括玩家的数量、玩家的数量以及最后的扑克牌列表，我不得不相应地改变游戏状态(我相信卡片-动作关系从代码中是足够清晰的，所以我不会在这里详细介绍)。

这个任务不是用OOP来完成的，它要求我具有某些功能。我把这些函数作为方法来实现。其中一些可能看起来过时了(即getWinner())，但我无法删除它们。

我想让你回顾一下我的代码并给出反馈(嗯，嗯，呃:P )。如果你坚持这几点，我会很高兴的：

我用了正确的OOP吗？
我处理好了例外情况吗？如果没有，我做错了什么？
我对std::move()的使用是否真的改变了任何东西并以任何方式提供了帮助？
在我的异常定义中，有一堆重复的代码，我能以某种方式修复它吗？

我的代码：

game.h:

#ifndef GAME_H
#define GAME_H

#include <vector>
#include <stack>
#include <string>
#include <fstream>
#include <algorithm>
#include <exception>
#include <stdexcept>

// Exception structs
class input_file_error : public std::exception
{
public:
    input_file_error(const char* message) : msg_(message) {}
    input_file_error(const std::string& message) : msg_(message) {}
    const char* what() const throw()
    {
        return msg_.c_str();
    }

protected:
    std::string msg_;
};

class game_runtime_error: public std::exception
{
public:
    game_runtime_error(const char* message) : msg_(message) {}
    game_runtime_error(const std::string& message) : msg_(message) {}
    const char* what() const throw()
    {
        return msg_.c_str();
    }

protected:
    std::string msg_;
};

// Only used together with game
struct Player
{
    std::string name;
    std::string surname;
    int score;
    friend std::ostream& operator<<(std::ostream& out, const Player& player);
};

class Game
{
private:
    // Current players
    std::vector<Player> m_players;

    // Current players that are out
    // Only the top player shall be accessed, the order
    // Shall not be changed
    std::stack<Player, std::vector<Player>> m_outPlayers;

    // Cards in game, game ends when no more cards are left
    std::string m_cardPlays;

    // Card symbols
    static const char cardAdd = '+';
    static const char cardRemove = '-';
    static const char cardNameSort = 'V';
    static const char cardSurnameSort = 'P';
    static const char cardScoreSort = '~';

    // Only used during initialisation
    bool isValidCard(const char card) const throw();
    Player getWinner() const throw();
    void sortByName() throw();
    void sortBySurname() throw();
    void sortByAlternatingScore() throw();
    // We can assume that this never fails
    void removeLastPlayer() throw();
    // If this fails, it shall do nothing
    void bringBackLastPlayer() throw();
    // Updates scores, never fails
    void updateScores() throw();
    // Prints the winner to the given file
    void printResults(const std::string& fName) const throw();

public:
    // Takes a file from which it should initialize the game
    // Throws exceptions on invalid initialisation
    Game(const std::string& fName);
    // Execute the game, return the winner
    // Throws if m_cardPlays is empty at the start of the execution
    void play(const std::string& fName);
    void getDataFromFile(const std::string& fName);

    // For debugging
    friend std::ostream& operator<<(std::ostream& out, const Game& cGame);
};

#endif // GAME_H

game.cpp:

#include "game.h"
#include <sstream>
#include <utility>

// Temporary, for debugging
#include <iostream>

std::ostream& operator<<(std::ostream& out, const Player& player)
{
    out << player.name << ' ' << player.surname << ' '
    << player.score << '\n';
    return out;
}

void Game::getDataFromFile(const std::string& fName)
{
    // Clear the old data
    m_players.clear();
    while(!m_outPlayers.empty()) {
        m_outPlayers.pop();
    }

    std::ifstream in(fName);
    if(!in) {
        throw input_file_error("Unable to open the file.");
    }

    int numOfPlayers;
    in >> numOfPlayers; in.ignore();

    if(!in) {
        throw input_file_error("The number of players is missing.");
    }

    if(numOfPlayers < 4) {
        throw input_file_error("Not enough players to play the game");
    }
    if(numOfPlayers > 20) {
        throw input_file_error("Too many players to play the game.");
    }

    int pCount(0);
    while(pCount < numOfPlayers && in) {
        std::string line;
        std::getline(in, line);

        // Skip empty lines
        if(line.empty()) {
            continue;
        }
        ++pCount;

        std::stringstream ss(line);
        Player tPlayer;
        ss >> tPlayer.name >> tPlayer.surname;

        // If a card list is used as player input
        // Also if only name/surname of a player is given
        if(tPlayer.surname.empty()) {
            throw input_file_error("Given and actual number of players does not match.");
        }
        // Default score
        tPlayer.score = 100;
        m_players.push_back(std::move(tPlayer));
    }

    if(!in) {
        throw input_file_error("Given and actual number of players does not match, card play list is missing.");
    }

    // Skip empty lines
    while(std::getline(in, m_cardPlays)) {
        if(!m_cardPlays.empty()) {
            break;
        }
    }

    if(m_cardPlays.empty()) {
        throw input_file_error("Card play list missing.");
    }

    // Cards are given in the order they are played
    std::reverse(m_cardPlays.begin(), m_cardPlays.end());

    bool playListFine = true;
    for(size_t i = 0; i < m_cardPlays.size(); ++i) {
        if(!isValidCard(m_cardPlays.at(i))) {
            playListFine = false;
            break;
        }
    }

    // If invalid card list was given
    if(!playListFine) {
        throw input_file_error("Invalid card play list is given.");
    }
}

Game::Game(const std::string& fName)
{
    getDataFromFile(fName);
}

std::ostream& operator<<(std::ostream& out, const Game& cGame)
{
    for(auto it = cGame.m_players.begin(); it != cGame.m_players.end(); ++it) {
        out << *it;
    }
    out << cGame.m_cardPlays << '\n';
    return out;
}

bool Game::isValidCard(const char card) const throw()
{
    switch(card)
    {
    case cardAdd:
    case cardNameSort:
    case cardScoreSort:
    case cardSurnameSort:
    case cardRemove:
        return true;
    default:
        return false;
    }
}

// Returns the player with the highest score. If scores of 2 players are equal, return the one higher in the list
Player Game::getWinner() const throw()
{
    const Player* winner = &m_players.at(0);
    for(auto it = m_players.begin() + 1; it != m_players.end(); ++it) {
        if(it->score > winner ->score) {
            winner = &(*it);
        }
    }
    return *winner;
}

void Game::sortByName() throw()
{
    std::sort(m_players.begin(), m_players.end(), [](const Player& p1, const Player& p2) { return p1.name < p2.name; });
}

void Game::sortBySurname() throw()
{
    std::sort(m_players.begin(), m_players.end(), [](const Player& p1, const Player& p2) { return p1.surname < p2.surname; });
}

// Is there a nicer way to do this?
void Game::sortByAlternatingScore() throw()
{
    std::sort(m_players.begin(), m_players.end(), [](const Player& p1, const Player& p2) {return p1.score > p2.score; });

    auto maxIt = m_players.begin();
    auto minIt = m_players.end() - 1;
    std::vector<Player> tVec;
    tVec.reserve(m_players.size());
    while(maxIt <= minIt) {
        tVec.push_back(*maxIt++);
        if(maxIt < minIt) {
            tVec.push_back(*minIt--);
        }
    }
    m_players = tVec;
}

// Removes last player from the list, adds it to outPlayers list
void Game::removeLastPlayer() throw()
{
    m_outPlayers.push(std::move(m_players.back()));
    m_players.pop_back();
}


/** Unclear conditions, how should this player be positioned in the
    list? Need to ask **/ // For now ignore this
// Removes player from outPlayers list, adds it to player list
void Game::bringBackLastPlayer() throw()
{
    if(m_outPlayers.size() == 0) {
        return;
    }
    m_players.push_back(std::move(m_outPlayers.top()));
    m_outPlayers.pop();
}

void Game::updateScores() throw()
{
    int scoreMultiplier = m_players.size();
    for(size_t i = 0; i < scoreMultiplier; ++i) {
        m_players.at(i).score += (scoreMultiplier - i) * 10;
    }
}

void Game::play(const std::string& fName)
{
    if(m_cardPlays.empty()) {
        throw game_runtime_error("Unable to play, empty card play list.");
    }

    while(!m_cardPlays.empty()) {
        char cCard = m_cardPlays.back();
        m_cardPlays.pop_back();

        switch(cCard)
        {
        case cardAdd:
            bringBackLastPlayer();
            break;
        case cardNameSort:
            sortByName();
            break;
        case cardRemove:
            removeLastPlayer();
            break;
        case cardScoreSort:
            sortByAlternatingScore();
            break;
        case cardSurnameSort:
            sortBySurname();
            break;
        // No need to check for card validity, already did that in
        // the constructor
        }
        updateScores();

        // For debugging
        std::cout << *this << '\n';
    }

    printResults(fName);
}

void Game::printResults(const std::string& fName) const throw()
{
    std::ofstream out(fName);
    out << getWinner();
}

main.cpp:

#include <iostream>
#include "game.h"

int main()
{
    const std::string inDat = "Duomenys.txt";
    const std::string outWin = "Laimetojas.txt";
    try
    {
        Game game(inDat);
        std::cout << game << '\n';
        game.play(outWin);
        game.getDataFromFile(inDat);
        game.play(outWin);
    }
    catch(input_file_error& e)
    {
        std::cout << "Error while operating the input file: \n"
        << e.what() << '\n';
    }
    catch(game_runtime_error& e)
    {
        std::cout << "Game error: \n"
        << e.what() << '\n';
    }
    catch(...)
    {
        std::cout << "Unknown exception.";
    }
}

编辑:我忘了添加输入文件.韦尔普，我凌晨3点发的，只是错过了一些东西.下面是：

Duomenys.txt

6

Blezas Paskalis

Petras Klaustukas

Balys Sruoga

Sarpis Idomutis

Informikas Baltutis

Konstitucija Balsute

V-~P~++V

编辑:根据要求，我写了一个简单的游戏规则摘要。

给出一堆卡片(输入文件的最后一行)，它们按给定的顺序播放。游戏结束时，不再剩牌。每回合，打一张牌。每张卡的功能：

'V‘-球员的名单是按球员的名字，更大的名字第一(安娜比鲍勃，等等)。
'P‘-球员的名单是按球员的姓氏，更大的姓氏第一(杰金斯之前祖赞等)。
'+‘-最后一个被移除的玩家回到游戏中(如果Johny被移除，那么Pete被移除，然后这张牌被打了，Pete回到游戏中)
“~”--球员名单按分数交替排列(不知道怎么说，先用最高分，然后是最小分，其余得分最高，其余得分最小，等等)。

每个回合后(一张牌后)，玩家的分数会根据这个算法更新：

第一名球员在名单中得到n*10分，其中n是目前在游戏中的玩家人数；
第二名球员在名单中获得(n-1)*10分；
第三名获得(n-2)*10分，等等。

胜利者是得分最高的球员。玩家的数量必须在3到20之间。我们可以假设，每当一张“-”牌被使用时，列表中至少还剩下一位玩家(牌牌中的“-”牌的数量低于玩家的数量)。起始球员列表与文件中提供的一样。如果在比赛结束时，多个人都有获胜的分数，那么在名单中排名较高的人就会获胜。

我希望我没有错过任何事。对不起，如果我的英语不好，请更正我的代码和我的语言^^。

c++11

game

c++

object-oriented

回答 1

Code Review用户

回答已采纳

发布于 2016-03-24 17:43:11

我看到了一些可以帮助你改进你的计划的东西。

保持实现细节私有

OOP的基石之一是信息隐藏的概念。也就是说，接口是公共的，但是实现通常是私有的。在这段代码中，它做得很好，但可以改进。特别是，Player类可以完全位于Game类中，因为正如注释所指出的那样，它只能在该类中使用。

不使用不推荐使用的特性

throw()动态异常规范将不再适用于C++17，并且已经成为有一段时间不受欢迎。新的机制是使用noexcept。不过，话虽如此，请看下一点。

不要过度指定您的接口

在这么多函数上使用throw()可能不是一个好主意，原因有二。首先，它要求您的接口永远不要抛出，不管您以后可能希望如何重新实现这些功能。在几乎每一种情况下，这都不是真正有帮助的，因为编译器通常可以自己判断函数是否为noexcept。其次，对于许多编译器来说，性能没有多大关系。有关此主题的更多视图，请参见这个问题。

使用范围-用于简化代码

由于您使用的是C++11，所以可以使用range-for来简化代码。例如，当前代码包括以下内容：

std::ostream& operator<<(std::ostream& out, const Game& cGame)
{
    for(auto it = cGame.m_players.begin(); it != cGame.m_players.end(); ++it) {
        out << *it;
    }
    out << cGame.m_cardPlays << '\n';
    return out;
}

可以将其简化为：

std::ostream& operator<<(std::ostream& out, const Game& cGame)
{
    for(const auto& player: cGame.m_players) 
    {
        out << player;
    }
    return out << cGame.m_cardPlays << '\n';
}

小心使用已签名的和未签名的

当前的代码包含以下功能：

void Game::updateScores() throw()
{
    int scoreMultiplier = m_players.size();
    for(size_t i = 0; i < scoreMultiplier; ++i) {
        m_players.at(i).score += (scoreMultiplier - i) * 10;
    }
}

但是，在某些平台上，size_t是无符号的，因此循环比较了一个无符号数字和一个有符号数字。我会避免这个问题，并将代码简化如下：

void Game::updateScores() 
{
    auto scoreAdder = m_players.size() * 10;
    for(auto& player : m_players) {
        player += scoreAdder;
        scoreAdder -= 10;
    }
}

请注意，这需要Player::operator+=的实现，但非常简单：

Player& operator+=(int scoreAdder) { 
    score += scoreAdder;
    return *this;
}

使用标准算法

使用getWinner()可以大大简化std::max_element代码：

Game::Player Game::getWinner() const noexcept
{
    return *std::max_element(m_players.begin(), m_players.end(), [](const Player& p1, const Player& p2) { return p1.score < p2.score; });
}

使用标准异常

不要使用当前代码中定义的两个异常类，只需使用std::runtime_error，这将消除冗余代码并简化。或者，如果确实需要区分自定义类，则从std::runtime_error派生它们。

class game_runtime_error: public std::runtime_error
{
public:
    game_runtime_error(const char *message) : std::runtime_error(message) {}
};

让编译器销毁对象

在getDataFromFile中，我们有以下代码：

// Clear the old data
m_players.clear();
while(!m_outPlayers.empty()) {
    m_outPlayers.pop();
}

但是，这几乎完全符合我们对没有参数的默认构造函数的期望。我会通过这样做来正规化：

{
    Game g;
    std::swap(*this, g);
}

大括号是用来告诉编译器，当该作用域结束时，可以自动删除g。如果不希望创建空Game，请将空构造函数设置为私有：

Game() {};

重新考虑接口设计

现在，Game::getDataFromFile()例程接受一个std::string，但是最好让它使用一个std::istream&。这样，就可以很容易地接受输入，即使是来自非文件的东西，比如来自std::cin。

为`Player`

定义构造函数

现在，Player对象是用Game中的代码实例化的。OOP设计最好是为Player设置一个构造函数，并将其委托给它。我是这样写的：

Player(std::string line) : score{100} {
    std::stringstream ss(line);
    ss >> name >> surname;
}

在实际的

中使用emplace_back

当前代码有以下一行：

m_players.push_back(std::move(tPlayer));

但是，有一个std::vector成员函数更好地表达了这一点。是emplace_back：

m_players.emplace_back(tPlayer);

避免在实际的

中使用break

使用break意味着控制流不同于通常的控制流，这使得代码比容易使用的替代程序更难阅读。例如，代码包括以下内容：

// Skip empty lines
while(std::getline(in, m_cardPlays)) {
    if(!m_cardPlays.empty()) {
        break;
    }
}

这条评论很有帮助，但我想我倾向于这样写循环：

while(std::getline(in, m_cardPlays) && m_cardPlays.empty())
{}

如果可能的话，

会提前失败，

Game的构造函数目前包括以下内容：

bool playListFine = true;
for(size_t i = 0; i < m_cardPlays.size(); ++i) {
    if(!isValidCard(m_cardPlays.at(i))) {
        playListFine = false;
        break;
    }
}

// If invalid card list was given
if(!playListFine) {
    throw std::runtime_error("Invalid card play list is given.");
}

这可以简化得多，因为一旦找到一个无效的卡片，继续查看卡就没有多大意义了：

for (const auto card : m_cardPlays) {
    if (!isValidCard(card)) {
        throw std::runtime_error("Invalid card play list is given.");
    }
}

执行就地重排，

要回答Game::sortByAlternatingScore()上面的评论中提出的问题，是的，有一个更好的方法。与分配单独的向量不同，操作完全可以在适当的地方完成：

void Game::sortByAlternatingScore() throw()
{
    std::sort(m_players.begin(), m_players.end(), [](const Player& p1, const Player& p2) {return p1.score > p2.score; });
    if (m_players.size() < 3) {
        return;
    }
    for (auto first=m_players.begin()+1; 
              first != m_players.end(); ++first) {
        std::reverse(first, m_players.end());
    }
}

票数 4

页面原文内容由Code Review提供。腾讯云小微IT领域专用引擎提供翻译支持

原文链接：

https://codereview.stackexchange.com/questions/123713

复制

相似问题

问自定义纸牌游戏
EN

我的代码：

game.h:

game.cpp:

main.cpp:

Duomenys.txt

回答 1

Code Review用户

保持实现细节私有

不使用不推荐使用的特性

不要过度指定您的接口

使用范围-用于简化代码

小心使用已签名的和未签名的

使用标准算法

使用标准异常

让编译器销毁对象

重新考虑接口设计

为`Player`

在实际的

避免在实际的

会提前失败，

执行就地重排，

社区

活动

圈层

关于

腾讯云开发者

热门产品

热门推荐

更多推荐

问自定义纸牌游戏EN

我的代码：

game.h:

game.cpp:

main.cpp:

Duomenys.txt

回答 1

Code Review用户

保持实现细节私有

不使用不推荐使用的特性

不要过度指定您的接口

使用范围-用于简化代码

小心使用已签名的和未签名的

使用标准算法

使用标准异常

让编译器销毁对象

重新考虑接口设计

为Player

在实际的

避免在实际的

会提前失败，

执行就地重排，

社区

活动

圈层

关于

腾讯云开发者

热门产品

热门推荐

更多推荐

问自定义纸牌游戏
EN

为`Player`