-3

我写了一个运行良好的代码。我只是想要额外的眼睛来突出每一件应该/可以改进的事情。我必须创建一个 student.dat 文件，并写入用户提供的数据（每个学生的姓名、年龄、gpa），然后关闭它，然后重新打开它进行阅读，显示学生的 gpa。Student 是一个 Class 对象。我只是想用 OOP 的概念来检查我在哪里（至少在那个问题上）。我正在使用 Dev-C++。代码如下：

#include <iostream>
#include <conio.h>
#include <fstream>
#include <string>
#include <iomanip>

#define W setw
using namespace std;

class Student
{
private:
    char name[40];
    int age;
    double GPA;

public:
    Student(){};
    void read();
    void show(char* ,int ,double ); 
    int writefile(ofstream &OS);
    double getgpa(double*, int );
    void readfile();
};



void Student::read(void)
{
int nbrSt=0;
cout<<"Please enter the information of the student: "<<endl;
cout<<"'y' to Continue, Ctrl+Z to exit! "<<endl;    
cout<<"Name, Age and GPA:\n";


ofstream OS("student.dat", ios::out);
while (cin>>name>>age>>GPA)
{       
        //writing in the file
        writefile(OS);
        nbrSt++;  //increment the number of students
        cout<<"Name, Age and GPA:\n";
}
OS.close();

}

int Student::writefile(ofstream & OS)
{

OS<<'\n'<<W(10)<<name<<W(6)<<age<<W(10)<<GPA;

 return 0;   
}

void Student::show(char* Name, int Age, double gpa)
 {
cout<<'\n'<<W(10)<<Name<<W(6)<<Age<<W(8)<<gpa<<endl;    

 }

double Student::getgpa(double* allGPA, int nbrgpa)
{
double sum=0;
int i =0;

 for ( i=0;i<nbrgpa; i++)
   sum+=allGPA[i];

 if(nbrgpa>0)
   return sum/nbrgpa;

 }


void Student::readfile()
{
char Name[30];
int Age;
double aveGPA, gpa;
    int nbrgpa=0;
double  allGPA[50];
int i=0;


ifstream IS("Student.dat", ios::in);
cout<<"reading from Binary file"<<endl;

if (IS.is_open())

    while(IS>>Name>>Age>>gpa)
    {   
        nbrgpa++;
        show(Name, Age, gpa);
        allGPA[i]=gpa;
        i++;    
}
  IS.close();


aveGPA=getgpa( allGPA, nbrgpa);

cout<<"Average GPA of students: "<<aveGPA<<endl;

}

int main(void)
{
Student S;

S.read();
S.readfile();

return 0;
}
4

2 回答 2

2 
Student(){};

是非法语法

Student(){}

是正确的。

void Student::read(void)

设计得很糟糕。Student::read 应该是一种读取一个学生的方法。多学生的阅读和学生的写作应该在其他功能或方法中。

int Student::writefile(ofstream & OS)

应该

int Student::writefile(ostream & OS)

所以它适用于各种流（不仅仅是文件流）。显然，您应该重命名该方法。举个例子，就叫它写吧。

double Student::getgpa(double* allGPA, int nbrgpa)

不应该是 Student 的成员，它应该是一个全局函数。

您的主要问题是面向对象的设计。你不应该只是将所有内容添加到Student课程中而不考虑你在做什么。Student 类中的方法应该是关于一个学生的，例如读取或写入一个学生的方法。其他所有内容都应该在全局函数中（或者如果您在程序中添加第二个类，则在其他类中）。

于 2013-04-26T10:52:43.310 回答
1

我只是想要额外的眼睛来突出每一件应该/可以改进的事情。

#define W setw

不要那样做。您可能认为它使使用 setw 的代码看起来更简单，但是查看您的代码的其他人将不得不搜索 W 解析的内容。

using namespace std;

不要全局声明 using namespace std。在小型项目中这不是什么大问题，但它使代码更难以重用。

你的类的接口是非标准的。考虑通过创建ostream << student运算符进行阅读，并通过创建“istream >> student”进行阅读。这尊重最小意外规则，并使您能够（例如）使用迭代读取一系列学生。

您需要更好的函数名称：

您的读取函数写入文件。在生产代码中看到这一点对我来说将是一个重要的 WTF 时刻。要么更改名称，要么更改功能。

您的 read 函数在学生实例 ( ) 上调用，Student s; s.read();但它不适用于实例。它实际上将一组记录传输/存储cin到一个文件中，并将调用它的实例设置为最后一条记录。如果在数据中途读取失败（即cin>>name>>age>>GPA正确获取名称但未获取年龄或 GPA），它将使调用它的实例处于无效状态。

考虑将您的代码分别从char name[40];和double* allGPA, int nbrgpa移到std::string和移出std::vector<double>。使用原始数据容易出错并且不必要地复杂。

还有很多话要说，但我想我已经给了你足够的:-)。

于 2013-04-26T11:09:10.407 回答