Calculadora de calificaciones (Nueva versión OOP de una anterior)
Estoy tratando de desarrollar una forma de pensar OOP. Esperaba que alguien tuviera la amabilidad de dedicar parte de su valioso tiempo a revisar mi programa OOP Calculadora de calificaciones. Como siempre, me gustaría saber qué he hecho bien, qué debería mejorar y cualquier sugerencia sobre cómo podría mejorar lo que tengo. Por cierto, tengo una clase llamada Clase. Probablemente debería prefijarlo con "cls" para no confundir. Trate este programa como se supone que debe ingresarse, no lo he verificado por error. El objetivo de este programa es desarrollarse en POO.
// 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();
}
Respuestas
Aquí hay algunas cosas que le ayudarán a mejorar su programa.
No mezcle E / S con operaciones de datos
En general, tener una std::getliney std::coutdentro de una clase de datos como Classno es una buena idea. Hace que sea más difícil reutilizar la clase. Mejor práctica es mantener los datos ( Student, Class, etc.) separada de obtener información de los usuarios. El patrón de diseño Modelo-Vista-Controlador suele ser útil para programas como este. Considere usar algo como una ConsoleMenuclase como en esta respuesta .
Use la semántica de movimiento cuando sea práctico
En la AddStudentfunción, tenemos esta línea:
students.push_back({ name, scoreGot });
Sería mejor usar lo emplace_backque le dice al compilador que no necesita construir y copiar, pero que es seguro construir el objeto en su lugar.
Repensar la interfaz de la clase
La Classclase tiene esta función miembro:
std::vector<Student>& accessStudents() { return students; }
Esta es una mala idea. Devuelve una referencia a un miembro de la clase interna. Piense en lo que sucede si Classse elimina la instancia pero alguna entidad externa todavía tiene una referencia a datos que ya no existen. El único lugar donde se utiliza está dentro GradeCalculator::averageGrade()y GradeCalculator::DislayResult()por lo que es un fuerte indicador de que algo no está bien en la interfaz de la clase. Sugeriría hacer de la averageGrade()función una Classfunción miembro.
Utilice valores predeterminados cuando sea sensato
El Studentconstructor es esencialmente el mismo que habría generado el compilador. Para reducir la posibilidad de error, simplemente se puede eliminar.
Piense en el usuario
No hay una forma obvia de salir del programa con elegancia. Sugeriría agregar un elemento de menú para eso. Además, es probable que cualquiera que esté usando el programa quiera ingresar a toda la clase de estudiantes y luego calcular los puntajes de toda la clase. Tener que seleccionar repetidamente "1. Agregar alumno ..." es un poco tedioso. Sería mejor que el programa recopilara información hasta que se ingresara un nombre vacío o tal vez "salir" y luego mostrar automáticamente las puntuaciones. Mejor aún sería permitir la entrada desde un archivo de texto.