一位初入门的新员工的代码分析（1）

    公司刚来的新员工，刚毕业，经过几个月的实践，完成了一个实训编码练习。通过对其代码的阅读，发现了一些问题。这些问题对于初学者来说，是比较容易犯的错误，而且以他们目前的经验来说，不容易发现或理解。对此进行专门的解读和分享，希望各位初学者能够有所启发，早日提高编码水平。

    任务比较简单，是完成一个学生管理系统，现在很多大学生都自己玩这样的系统。要求使用链表、XML等考核点。

以下是其写的学生链表类头文件：

#include "Student.h"
#include "XMLWrapper.h"
struct StudentNode
{
 CStudent m_Student;
 StudentNode *next;
};
class CStudentList
{
public:
CStudentList();
~CStudentList();
 //初始化学生链表
void InitializeStudentList(int nSize,CStudent *Student);
    void InitializeStudentList();
 //添加学生
 void AddStudent(CStudent Student);
//删除学生
void DeleteStudent(CString StudentID);
//清空学生链表
void ClearStudentList();
//通过学号查找学生
CStudent* SelectStudent(CString strStudentID);
//查找班级内的学生
BOOL  SelectCalssStudent(int *Count,CStudent **Student,CString strMajor,CString strGrade,CString strClass);
//保存学生信息
BOOL SaveStudentInfomation(TiXmlElement *RootElement,CString strXML,CXMLWrapper *pTinyXML);
//检查学生链表是否相等
BOOL CheckStudentList(CStudentList *pStudentList);
private:
StudentNode *m_head;
};

对于这个头文件定义，初学者可以从自己目前的经验来看看有什么优缺点。然后再看下面的分析，可能会更有价值一些。

请

先

思

考

不

要

偷

看

好吧，下面分析一下从这个头文件中看到的不足。

1。 变量命名风格不统一

– CString StudentID     VS  CString strStudentID

– CStudent *Student    VS  CStudent **Student

可读性差，读者难以通过变量名来容易辨识变量类型，很容易混淆

建议：实在不知道该怎么办的话，一是找个编码规范学一学，二是看看MFC吧。

2。 变量名称无意义

– StudentNode *m_head

m_head能代表什么？不能达到顾名思义的效果。

建议：最优秀的注释就是不用写注释。不用写是指代码本身就可以自解释。

3。函数名有单词错误

– Select Calss Student

班级--Class

建议：实在不清楚的话，可以查查字典啊。

4。 返回值类型不合理

– void InitializeStudentList(int nSize,CStudent *Student);

– void AddStudent(CStudent Student);

操作是否成功，如何让用户了解？

建议：业务功能性操作，尽量考虑保留返回值，是否成功对外部的处理有较大的影响。

5。 参数个数太多

– BOOL  Select Calss Student(int *Count,CStudent **Student, CString strMajor,CString strGrade,CString strClass );

未来班级属性增加怎么办？

建议：关系比较紧密地参数，且数量较多，有扩展可能性的，建议使用结构，可保持接口不变

           另外，常见的OUT型参数，一般放在函数参数的末尾

6。类的职责不明确

– BOOL SaveStudentInfomation(TiXmlElement *RootElement,CString strXML,CXMLWrapper *pTinyXML);

学生列表类是否负责学生信息的读写，还只是作为一个学生对象的容器？两种方案似乎都只完成了部分功能，封装还不够。

如果仅作为容器，是抽象的，与业务无关；而读写，属业务范畴

如果负责读写，那么应该有信息的读取处理部分，而不是只有保存部分

扩展考虑，存储未来可能存在不同的存储方式，这里直接固化为XML文件格式存储，未来无法扩展。

如果只作为容器，那么文件存储类应该移除。为了支持外部遍历容器进行相关处理，类应该增加遍历机制，如Prev()，Next()，Head()，Tail()等

另外，作为容器类，似乎还应该有修改学生信息的函数

相关性：设计模式中，有 单一职责原则 ；有 迭代器模式 。

其它建议：可将节点中的学生对象定义为指针类型