成績計算機(以前の1つからの新しいOOPバージョン)

Dec 19 2020

私はOOPの考え方を開発しようとしています。私は、誰かが私の成績計算機OOPプログラムをレビューするために貴重な時間を割いてくれることを望んでいました。いつものように、私は自分がうまくやったこと、何を改善すべきか、そして私が持っているものをおそらく改善する方法についての提案を知りたいですか?ちなみに、Classというクラスがあります。混乱しないように、接頭辞「cls」を付ける必要があります。このプログラムは入力されたはずのプログラムとして扱ってください。エラーチェックはしていません。このプログラムのポイントは、OOPで開発することです。

   // Task 1.cpp : This file contains the 'main' function. Program execution begins and ends there.


#include <iostream>
#include <vector>
#include <algorithm>
#include <functional>
#include <numeric>
#include <string>
class TestPaper
{
public:
    int m_scoreOutOf;  
    bool checkBoundary(int value, int boundary) {
        if (value < 0 || value > boundary) {
            std::cout << "Score must be between " << " 0 and " << boundary << ". Please try again.\n";
            return false;
        }
        return true;
    }

};
class Student {
private:
    std::string m_name;
    int m_scoreGot;
public:
   
    Student(std::string name, int scoreGot)
        :m_name(name), m_scoreGot(scoreGot){}

    std::string getName() const { return m_name; }
    int getScoreGot() const { return m_scoreGot; }
};

class Class {
private:
    std::vector<Student>students;
public:
    void AddStudent(TestPaper& testPaper) {
        std::string name = "";
        int scoreGot = 0;
        std::cout << "Enter student name: ";
        std::getline(std::cin >> std::ws, name);
        do
        {
            std::cout << "\nWhat did " << name << " score?\nEnter a score between 0 and "
                << testPaper.m_scoreOutOf << ": ";
            std::cin >> scoreGot;
        } while (testPaper.checkBoundary(scoreGot, testPaper.m_scoreOutOf) == false);
        students.push_back({ name, scoreGot });
    }

    std::vector<Student>& accessStudents() { return students; }
};

class GradeCalculator {
    TestPaper m_testPaper;
    Class m_ClassOfStudents;
public:
    GradeCalculator(TestPaper testPaper, Class classOfStudents) :m_testPaper(testPaper), m_ClassOfStudents(classOfStudents) {}
    void DisplayMenu() {

        std::cout << "\n1. Add student and their grade\n";
        std::cout << "2. Calculate class score\n";
        std::cout << "3. Modify testpaper (haven't implemented this yet)\n";
    }
    

    double averageGrade() {
        auto sum = std::transform_reduce(m_ClassOfStudents.accessStudents().begin(), m_ClassOfStudents.accessStudents().end(), 0.0, std::plus<>(),
            [&](auto& student) { return calculateGradePercentage(student); });
        return sum / m_ClassOfStudents.accessStudents().size();
    }
    double calculateGradePercentage(Student &student)
    {
        return static_cast<double>(student.getScoreGot()) / static_cast<double>(m_testPaper.m_scoreOutOf) * 100;
    }
    void DisplayResult() {
        for (auto& student : m_ClassOfStudents.accessStudents()) {
            std::cout << "Percentage scores are: \n";
            std::cout << student.getName() << ": " << calculateGradePercentage(student) << "%\n";
        }
        std::cout << "Average grade perecentage: " << averageGrade() << "%\n";
    }
    void runProgram() {

        int menuChoice = 0;
        while (true)
        {
            DisplayMenu();
            std::cout << "\nEnter a choice from the menu: ";
            std::cin >> menuChoice;
            switch (menuChoice)
            {
            case 1:
                m_ClassOfStudents.AddStudent(m_testPaper);
                break;
            case 2:
                DisplayResult();
                break;
            default:
                std::cout << "Invalid choice!\n";
            }
        }
    }
};

int main()
{
    TestPaper testPaper({ 20 });
    Class classOfStudents;
    GradeCalculator calculator(testPaper, classOfStudents);
    calculator.runProgram();
   
}

回答

6 Edward Dec 19 2020 at 09:35

プログラムを改善するのに役立つことがいくつかあります。

I / Oとデータ操作を混在させないでください

一般的に言えば、std::getlineなどstd::coutのデータクラス内にとを含めることClassはお勧めできません。クラスの再利用が難しくなります。より良い練習が(データを保持することでStudentClassユーザからの入力を得ることから切り離すなど、)。モデル-ビュー-コントローラのデザインパターンは、多くの場合、このようなプログラムのために有用です。この回答のようにConsoleMenuクラスのようなものを使用することを検討してください。

実用的な場合は移動セマンティクスを使用する

ではAddStudent機能、我々はこのラインを持っています:

students.push_back({ name, scoreGot });

emplace_backコンパイラに、構築およびコピーする必要はないが、オブジェクトを所定の場所に構築しても安全であることを通知するを使用することをお勧めします。

クラスインターフェイスを再考する

Classクラスは、このメンバ関数があります。

std::vector<Student>& accessStudents() { return students; }

これは悪い考えです。内部クラスメンバーへの参照を返します。Classインスタンスが削除されても、一部の外部エンティティが存在しなくなったデータへの参照を保持している場合はどうなるか考えてみてください。それが使用される唯一の場所は内部でGradeCalculator::averageGrade()あり、GradeCalculator::DislayResult()それはクラスインターフェースに何かが正しくないことを示す強力な指標です。averageGrade()関数をClassメンバー関数にすることをお勧めします。

賢明な場合はデフォルトを使用する

Studentコンストラクタは、基本的に、コンパイラによって生成されていたであろうものと同じです。エラーの可能性を減らすために、それは簡単に排除することができます。

ユーザーについて考える

プログラムを正常に終了する明確な方法はありません。そのためのメニュー項目を追加することをお勧めします。また、実際にプログラムを使用している人は、クラス全体の学生を入力してから、クラス全体のスコアを計算したいと思う可能性があります。「1.生徒を追加...」を繰り返し選択するのは少し面倒です。空の名前または「終了」が入力されるまでプログラムに入力を収集させてから、スコアを自動的に表示する方がよい場合があります。さらに良いのは、テキストファイルからの入力を許可することです。