修改后的基于控制台的测试应用程序

Question

这个问题是我之前问题的后续，链接到帖子可以找到这里。我真的很感谢评论员，特别是爱德华的评论。他们强调了在重构代码基时考虑到的要点。

概述

这个项目是一个基于控制台的测试应用程序.它可以由一个xml文件和一个测试对象的std::vector来构造。Quiz Application被设计为Quiz类的用户可以编写测试应用程序的方式之一。这暗示Quiz Application可能以另一种方式编程，这取决于用户的喜好。

AIM

1. 我的目标是练习面向对象的编程.
2. 我的目标是提高我的能力，以产生健壮和可用的类。
3. 我的目标是巩固我对STL容器和通用算法的知识。

主要关注事项

我不知道这是否适合于代码评审站点，或者可能应该发布在不同的堆栈交换站点上。我计划持久化QuizApplication类型的对象。SOLID设计原则要求类做一件事，并且做得很好。到目前为止，QuizApplication会生成测试和等级测试，它是否有必要将其类型的对象持久化，或者是否应该将作业转移到不同的类，同时铭记存储设备可能在不久的将来发生变化？

Quiz.h

#ifndef QUIZ_H_
#define QUIZ_H_

#include <iostream>
#include <vector>
#include <string>
#include <map>

struct Quiz 
{
    friend std::ostream& operator<<( std::ostream &os, const Quiz &quiz );
    Quiz();
    Quiz( const std::string &question, const std::vector<std::string>& options, \
        const std::string& answer, const std::vector<std::string>& tag );
    bool operator==( const Quiz& quiz );
    bool operator==( const Quiz& quiz ) const;
    std::string question_;
    std::string answer_;
    std::vector<std::string> tag_;
    std::map<char, std::string, std::less<char>> options_;
};

#endif

Quiz.cpp

#include "Quiz.h"

#include <iostream>

Quiz::Quiz( const std::string &question, const std::vector<std::string>& options, \
const std::string& answer, const std::vector<std::string>& tag )
     : question_{ question }, answer_{ answer }, tag_{ tag } 
    {
        int alphabelt = 65;
        for( const auto item : options )
            options_.insert( { char( alphabelt++ ) , item } );
    } 

std::ostream& operator<<( std::ostream &os, const Quiz &quiz )
{
    for( const auto &item : quiz.tag_ )
        os << "[" << item << "]"; 

    os << '\n' << quiz.question_ << '\n';

    for( const auto item : quiz.options_ )
        os << item.first << ". " << item.second << '\n';

    return os;
}

bool Quiz::operator==( const Quiz& quiz )
{
    if( question_ != quiz.question_ ) return false;
    if( answer_ != quiz.answer_ ) return false;
    if( options_ != quiz.options_ ) return false;
    if( tag_ != quiz.tag_ ) return false;

    return true;
}

bool Quiz::operator==( const Quiz& quiz ) const
{
    if( question_ != quiz.question_ ) return false;
    if( answer_ != quiz.answer_ ) return false;
    if( options_ != quiz.options_ ) return false;
    if( tag_ != quiz.tag_ ) return false;

    return true;
}

QuizApplication.h

#ifndef QUIZAPPLICATION_H_
#define QUIZAPPLICATION_H_

#include "Quiz.h"

#include <iostream>
#include <vector>
#include <unordered_set>
#include <map>

struct QuizHash
{
    size_t operator()( const Quiz &quiz ) const
    {
       return std::hash<std::string>()( quiz.question_ );
    }
};

class QuizApplication
{
    friend std::ostream& operator<<( std::ostream &os, const QuizApplication& app );

    public:
        QuizApplication( const std::vector<Quiz>& quiz )
        : quiz_container_{ quiz } {}
        explicit QuizApplication( const std::string &filename ) { read_xml( filename); };
        QuizApplication& add_quiz( const Quiz& quiz );
        QuizApplication& remove_quiz( const Quiz& quiz );
        void start_quiz() const;
        std::vector<Quiz>::iterator get( Quiz& quiz );
        std::vector<Quiz>::const_iterator get( const Quiz& quiz ) const;

        static int number_of_quiz_to_ask;

    private:
        std::vector<Quiz> quiz_container_;

        void read_xml( const std::string& filename );
        std::unordered_set<Quiz, QuizHash> generate_quiz_set() const;
        unsigned grade_quiz( std::multimap<char, Quiz> &quiz_choice ) const;
};

#endif

QuizApplication.cpp

#include "QuizApplication.h"

#include <iostream>
#include <algorithm>
#include <random>
#include <map>

#include <boost/property_tree/ptree.hpp>
#include <boost/property_tree/xml_parser.hpp>

int QuizApplication::number_of_quiz_to_ask = 0;

void QuizApplication::read_xml( const std::string& filename )
{
    namespace pt = boost::property_tree;

    pt::ptree tree;
    pt::xml_parser::read_xml( filename, tree );

    Quiz quiz_obj 
    {   "",
        {},
        "",
        {}
    };
    
    for( auto &quiz_item : tree.get_child( "quizlist" ) )
    { 
        int alphabelt = 65;
        if( quiz_item.first == "quiz" )
        {
            quiz_obj.question_ = quiz_item.second.get<std::string>("question");
            quiz_obj.answer_ = quiz_item.second.get<std::string>("answer");
            
            auto option_list = quiz_item.second.get_child("optionlist");
            for( auto &item : option_list )
            {
                quiz_obj.options_.insert( { char( alphabelt++ ), item.second.data() } );
            }

            auto tag_list = quiz_item.second.get_child("taglist");
            for( auto &item : tag_list )
            {
                quiz_obj.tag_.push_back( item.second.data() );
            }
        }
        quiz_container_.push_back( quiz_obj );
        quiz_obj.options_ = {};
        quiz_obj.tag_ = {};
    }
}

QuizApplication& QuizApplication::add_quiz( const Quiz& quiz )
{
    quiz_container_.push_back( quiz );

    return *this;
}

QuizApplication& QuizApplication::remove_quiz( const Quiz& quiz )
{ 
    auto iter = get( quiz );
    if( iter != quiz_container_.cend() )
        quiz_container_.erase( iter );

    return *this;
}

std::vector<Quiz>::iterator QuizApplication::get( Quiz& quiz )
{
    auto iter = std::find_if( quiz_container_.begin(), quiz_container_.end(), \
                            [quiz]( Quiz& this_quiz ) { return ( this_quiz == quiz ); } );
    return iter;
};

std::vector<Quiz>::const_iterator QuizApplication::get( const Quiz& quiz ) const
{
    auto iter = std::find_if( quiz_container_.cbegin(), quiz_container_.cend(), \
                            [quiz]( const Quiz& this_quiz ) { return ( this_quiz == quiz ); } );
    return iter;
}

std::ostream& operator<<( std::ostream &os, const QuizApplication& app )
{
    for( const auto& item : app.quiz_container_ )
        os << item << '\n';

    return os;
}

void QuizApplication::start_quiz() const
{
    std::unordered_set<Quiz, QuizHash> quiz_set = generate_quiz_set();
    std::multimap<char, Quiz> quiz_choice;

    for( const auto &item : quiz_set )
    {
        std::cout << '\n' << item << "? ";
        char choice;
        std::cin >> choice;
        quiz_choice.insert( { toupper( choice), item } );
    }

    unsigned score = grade_quiz( quiz_choice );
    char choice;
    std::cout << "Do you want your grade? (y/n)";
    std::cin >> choice;
    if( choice == 'y' ) std::cout << "Your score is " << score << std::endl;
    else std::cout << "See you some other time" << std::endl;
}

std::unordered_set<Quiz, QuizHash> QuizApplication::generate_quiz_set() const
{
    std::unordered_set<Quiz, QuizHash> quiz_set;
    std::default_random_engine engine( static_cast<unsigned>( time( nullptr) ) );
    std::uniform_int_distribution<unsigned> random_int( 0, quiz_container_.size() - 1 );

    for( int i = 0; i != number_of_quiz_to_ask; )
    {
        unsigned index = random_int( engine );
        auto insert_return = quiz_set.insert( quiz_container_[ index ] );
        if( insert_return.second )
            ++i;
        else 
            continue;
    }

    return quiz_set;
}

unsigned QuizApplication::grade_quiz( std::multimap<char, Quiz> &quiz_choice ) const
{
    unsigned score = 0;
    for( const auto& item : quiz_choice )
    {
        if( item.second.options_.at( item.first ) == item.second.answer_ ) 
            ++score;
    }
    return score;
}

quiz.xml

<quizlist>
    <quiz>
        <question>Pointer-based code is complex and error-prone—the slightest omissions or oversights
can lead to serious memory-access violations and memory-leak errors that the compiler
will warn you about.</question>
        <optionlist>
            <option>True</option>
            <option>False</option>
        </optionlist>
        <answer>False</answer>
        <taglist>
            <tag>C++</tag>
            <tag>Programming</tag>
        </taglist>
    </quiz>

    <quiz>
        <question>deques offer rapid insertions and deletions at front or back and direct access to any element.</question>
        <optionlist>
            <option>False</option>
            <option>True</option>
        </optionlist>
        <answer>True</answer>
        <taglist>
            <tag>C++</tag>
            <tag>Programming</tag>
        </taglist>
    </quiz>
</quizlist>

main.cpp

#include <iostream>

#include "Quiz.h"
#include "QuizApplication.h"

int main()
{
    QuizApplication::number_of_quiz_to_ask = 2;

    Quiz quiz_obj
    {
        "What is my name?",
        { "Samuel", "Kingsley", "Paul", "John" },
        "Samuel",
        { "Name" }
    };

    QuizApplication app { "quiz.xml" };
    app.add_quiz( quiz_obj );
    app.start_quiz();
}

Answer 1

Quiz类：

class Quiz --我可能会把这个类叫做Question。在英语中，“小测验”是一套完整的问题。

Quiz();

它看起来像是声明了默认构造函数，但没有定义。我们应该做Quiz() { }或Quiz() = default;

Quiz( const std::string &question, const std::vector<std::string>& options, \
    const std::string& answer, const std::vector<std::string>& tag );

将函数定义拆分到多行是很好的。没有必要使用\。

bool operator==( const Quiz& quiz );
bool operator==( const Quiz& quiz ) const;

我们不需要这两个。const成员函数可以在const和非const Quiz对象上调用，因此我们只需要const版本。

或者，我们可以使它成为一个接受两个Quiz对象的免费函数：

bool operator==(const Quiz& a, const Quiz& b);

我们将答案字符串存储在Quiz中两次--一次在answer_变量中，一次在options_映射中。我们只需要将正确答案的char键存储在answer_变量中。

    int alphabelt = 65;
    for( const auto item : options )
        options_.insert( { char( alphabelt++ ) , item } );

错误:在英语中是alphabet。

我们可以避免从int到char的转换，并通过执行：char alphabet = 'A';使其更加清晰。char也是一个整数类型，所以我们可以增加它。

我们可能希望在基于范围的for循环中使用const auto& item，而不是const auto item，以避免不必要的复制。

因为我们使用字母表字母来指问题，所以我们应该在某个地方检查一下，在测验中我们没有比字母表中的字符更多的问题。

QuizHash类：

使用整个问题字符串的散列作为存储键可能不是一个好主意。(计算哈希值会很慢，我们必须知道整个问题字符串才能找到问题)。

我不认为set在这里是必要的，我们只需要一个vector。

QuizApplication类：

    std::vector<Quiz>::iterator get( Quiz& quiz );
    std::vector<Quiz>::const_iterator get( const Quiz& quiz ) const;

这些函数只需要在QuizApplication中使用(外部代码无法访问quiz_container_)，因此它们可以是私有的。

    static int number_of_quiz_to_ask;

number_of_questions_to_ask将是一个更准确的名字。

Quiz quiz_obj 
{   "",
    {},
    "",
    {}
};

我们可以使用默认构造函数(假设它是定义的-参见上面)：Quiz quiz_obj;

    int alphabelt = 65;

与Quiz类相同(alphabet，使用char -参见上文)。

    quiz_obj.options_ = {};
    quiz_obj.tag_ = {};

我们可以通过在循环中声明quiz_obj来避免在每个循环迭代中重置变量。

auto iter = std::find_if( quiz_container_.begin(), quiz_container_.end(), \
                        [quiz]( Quiz& this_quiz ) { return ( this_quiz == quiz ); } );
return iter;

我们有一个用于operator==的Quiz，因此我们应该能够使用std::find(quiz_container_.begin(), quiz_container_.end(), quiz);。

同样，没有必要使用\。

生成测试集

std::unordered_set<Quiz, QuizHash> QuizApplication::generate_quiz_set() const

在生成问题集时，我们不希望复制所有的Quiz数据。我们可以在quiz_container_中生成一组随机索引，如下所示：

std::vector<std::size_t> QuizApplication::generate_quiz_set() const
{
    std::vector<std::size_t> out(quiz_container.size(), 0);
    std::iota(out.begin(), out.end(), 0); // out = { 0, 1, 2, 3 ... quiz_container.size() - 1 }
    
    std::default_random_engine engine(static_cast<unsigned>(time(nullptr)));
    std::shuffle(out.begin(), out.end(), engine); // shuffle the indices
    
    out.resize(std::min(number_of_quiz_to_ask, out.size())); // chop off the ones we don't need
    
    return out;
}

运行测试的

：

void QuizApplication::start_quiz() const
{
    std::unordered_set<Quiz, QuizHash> quiz_set = generate_quiz_set();
    std::multimap<char, Quiz> quiz_choice;

    for( const auto &item : quiz_set )
    {
        std::cout << '\n' << item << "? ";
        char choice;
        std::cin >> choice;
        quiz_choice.insert( { toupper( choice), item } );
    }

    ...

我们应该检查这个问题的choice字符是一个有效的选项，如果不是，再次询问用户。目前，我们将抛出一个异常时，评分测试后。

再说一遍，我认为我们不需要set或multimap。如果我们使用一个vector来存储生成的测试集，我们可以简单地使用另一个vector来存储选择：

void QuizApplication::start_quiz() const
{
    std::vector<std::size_t> quiz_set = generate_quiz_set();
    std::vector<char> quiz_choices;
    
    for( const auto &item : quiz_set )
    {
        std::cout << '\n' << item << "? ";
        char choice;
        std::cin >> choice;
        // todo: check choice is a valid option!!!
        quiz_choices.push_back(choice);
    }
    
    ...

现在，quiz_choices[i]包含对quiz_set[i]的响应，其中存储在quiz_set[i]中的值是对quiz_container_中Quiz的索引。因此，现在标记测试的内容如下：

unsigned score = 0;
for (auto i = std::size_t{ 0 }; i != quiz_choices.size(); ++i)
{
    auto const& choice = quiz_choices[i];
    auto const& index = quiz_set[i];
    
    if (choice == quiz_container_[index].answer_) // assuming we change answer_ to be a `char` instead of the full string
        ++score;
}