软件工程第5次作业(第2次结对作业)——驾驶员与领航员
项目地址
此项目的Github地址为:
Origin分支 - 拥有者:Rafael Gu
驾驶员:顾苡铭
Fork分支 - 拥有者:Oberon Zheng
领航员:郑竣太
题目要求
刚开始上课的时候介绍过一个小学四则运算自动生成程序的例子,请实现它,要求:
- 能够自动生成四则运算练习题
- 可以定制题目数量
- 用户可以选择运算符
- 用户设置最大数(如十以内、百以内等)
- 用户选择是否有括号、是否有小数
- 用户选择输出方式(如输出到文件、打印机等)
- 最好能提供图形用户界面(根据自己能力选做,以完成上述功能为主)
代码审查(Code Investigation)
审查基本信息
- 功能模块名称:表达式生成器
- 审查人:郑竣太
- 审查日期:2019年4月23日
- 代码名称:main.cpp
- 代码作者:顾苡铭
声明工作与代码文件管理(Declaration & Code Management)
审查项 | 重要 | 评估 | 备注 |
---|---|---|---|
using 是否包含了所有可能使用的库? |
√ | √ | |
using 是否包含多余的库(未使用的) |
× | √ | 有些using 由项目模板自动生成 |
版权和版本声明是否完整? | × | × | 此代码用于课程设计,没有重视版本控制 |
类代码文件名与类的名是否保持一致 | × | √(Partly) | 自定义类库中一个cs文件包含了多个类,这些类之间存在继承和引用关系 |
是否通过提出公共子式的方式将一些并列的作用域下同种用途的局部变量声明提到作用域外 | × | √ | |
设计器文件与窗体代码是否保持对应关系并一致 | √ | √ | |
是否擅自修改了设计器代码(本应当由设计器自动生成) | √ | × |
代码风格(Code Style)
审查项 | 重要 | 评估 | 备注 |
---|---|---|---|
空行是否得体? | × | √ | |
长行拆分是否得体? | × | - | |
{ 和 } 是否各占一行并且对齐于同一列? |
× | √ | |
一行代码是否只做一件事?如只定义一个变量,只写一条语句。 | √ | √ | |
if 、for 、while 、do 等语句自占一行,不论执行语句多少都要加 {} 。 |
√ | √ | |
注释是否有错误或者可能导致误解? | √ | × | |
类结构的public, protected, private顺序是否在所有的程序中保持一致? | √ | × |
命名规范(Naming Convention)
审查项 | 重要 | 评估 | 备注 |
---|---|---|---|
命名规则是否与所采用的操作系统或开发工具的风格保持一致? | √ | × | 窗体控件的名称不符合IDE的提倡格式(提倡PascalCase但使用了camelCase)但仍然符合某种规范 |
标识符是否直观且可以拼读? | × | √ | 但缩写较多 |
标识符的长度应当符合“min-length && max-information”原则? | × | √ | 仅限于满足上一条的内容 |
程序中是否出现相同的局部变量和全部变量? | √ | √ | 局部变量名称可能和某函数的参数名相同,但这不影响 |
类名、函数名、变量和参数、常量的书写格式是否遵循一定的规则? | × | √ | |
静态变量、全局变量、类的成员变量是否加前缀? | × | × |
表达式与基本语句 (Expressions & Statements)
审查项 | 重要 | 评估 | 备注 |
---|---|---|---|
如果代码行中的运算符比较多,是否已经用括号清楚地确定表达式的操作顺序? | √ | √ | |
是否编写太复杂或者多用途的复合表达式? | × | × | |
是否将复合表达式与“真正的数学表达式”混淆? | × | × | |
是否用隐含错误的方式写if 语句? |
√ | × | |
case 语句的结尾是否忘了加break ? |
√ | × | C#中case 后不加break 是语法错误 |
是否忘记写switch 的default 分支? |
√ | × | default 分支用于抛出异常 |
常量(Constants)
审查项 | 重要 | 评估 | 备注 |
---|---|---|---|
是否使用含义直观的常量来表示那些将在程序中多次出现的数字或字符串? | × | × |
函数(Functions)
审查项 | 重要 | 评估 | 备注 |
---|---|---|---|
参数的书写是否完整?不要贪图省事只写参数的类型而省略参数名字。 | × | √ | C#不允许省略参数名 |
参数命名、顺序是否合理? | × | √ | |
参数的个数是否太多? | × | × | |
是否使用类型和数目不确定的参数? | × | √ | 使用了param 参数表 |
函数名字与返回值类型在语义上是否冲突? | × | × | |
是否将正常值和错误标志混在一起返回?正常值应当用输出参数获得,而错误标志用return语句返回。 | × | √ | 这些函数没有返回错误标志,正常值直接返回得出,发生错误直接抛出异常 |
在函数体的“入口处”,是否用assert 对参数的有效性进行检查? |
√ | × | 没有使用assert |
滥用了assert ? 例如混淆非法情况与错误情况,后者是必然存在的并且是一定要作出处理的。 |
× | - | 同上 |
是否在发生预料之外的情形下抛出了异常 | √ | √ |
内存管理(Memory Management)
审查项 | 重要 | 评估 | 备注 |
---|---|---|---|
是否忘记为数组和动态内存赋初值?(防止将未被初始化的内存作为右值使用) | √ | - | 同上 |
数组或容器的下标是否越界? | √ | × | |
动态内存的申请与释放是否配对?(防止内存泄漏) | √ | - | C#存在GC机制 |
是否有效地处理了“内存耗尽”问题? | √ | × | 但判断了耗尽情形 |
是否对局部生成的对象使用了using 句 |
× | × |
C#函数特性
审查项 | 重要 | 评估 | 备注 |
---|---|---|---|
重载函数是否有二义性? | √ | × | |
是否混淆了成员函数的重载、覆盖与隐藏? | √ | × | |
运算符的重载是否符合制定的编程规范? | √ | - | 没有重载运算符 |
传引用时是否正确地使用了ref 和out |
√ | - | 没有传引用 |
提供默认参数的参数是否被列到最后 | √ | √ | |
动态参数列表是否被列到最后 | √ | √ |
类(Classes)
审查项 | 重要 | 评估 | 备注 |
---|---|---|---|
构造函数中是否遗漏了某些初始化工作? | √ | × | |
是否正确地使用构造函数的初始化表? | √ | √ | |
是否错写、错用了构造函数和? | √ | × | |
是否违背了继承和组合的规则? | √ | × | |
是否使用了属性(Properties)封装了私有的成员或只读的值 | √ | √ | 但不完全 |
是否使用单例模式替代静态类 | × | × | |
是否混淆使用了virtual 和abstract |
√ | × |
其它常见问题(Miscellaneous)
审查项 | 重要 | 评估 | 备注 |
---|---|---|---|
变量的数据类型有错误吗? | √ | × | |
存在不同数据类型的赋值吗? | √ | × | |
存在不同数据类型的比较吗? | √ | × | |
变量的初始化或缺省值有错误吗? | √ | × | |
变量发生上溢或下溢吗? | √ | × | |
变量的精度够吗? | √ | √ | |
由于精度原因导致比较无效吗? | √ | × | |
表达式中的优先级有误吗? | √ | × | |
逻辑判断结果颠倒吗? | √ | × | |
循环终止条件是否正确? | √ | √ | |
忘记进行错误处理吗? | √ | × | |
错误处理程序块一直没有机会被运行? | √ | √ | 部分处理程序没有执行机会 |
错误处理程序块本身就有问题吗?如报告的错误与实际错误不一致,处理方式不正确等等 | √ | × | |
文件以不正确的方式打开吗? | √ | × | |
没有正确地关闭文件吗? | √ | × | |
发生类型强转时,是否使用类推荐的转换函数或装拆箱替代强转 | × | √ | |
事件订阅时是否充分利用EventArgs类型对象进行数据传递 | √ | × | 事件订阅没有传递任何数据 |
是否使用了非安全编程代码(例如在C#中使用指针) | √ | × | .NET托管下没有启用非安全代码 |
是否对可能发生溢出的情形指定了uncheck ? |
√ | × |
单元测试(Unit Testing)
由于WinForm的代码大部分由IDE的设计器自动生成,因此这里着重测试类库MyExpression
的代码
功能类库MyExpression
包含两个类,其一是用于表示表达式的ExprBase
类及其附属的派生类,其二是用于生成表达式的ExprBuilder
类。
using Microsoft.VisualStudio.TestTools.UnitTesting;
using MyExpression;
using System;
using System.Text.RegularExpressions;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
namespace MyExpression.Tests
{
[TestClass()]
public class ExprBuilderTests
{
[TestMethod]
public void AtomExprTest()
{
AtomExpr aexp = new AtomExpr(12.5);
Assert.IsNotNull(aexp);
Assert.AreEqual(aexp.val, 12.5, double.Epsilon);
Assert.AreEqual(aexp.Sign, 1);
Assert.AreEqual(aexp.Priority, int.MaxValue);
Assert.IsTrue(aexp.Associative);
Assert.IsTrue(aexp.IsNaturalOrder);
Assert.AreEqual(aexp.ToString(), "12.5");
}
[TestMethod]
public void ExprTest()
{
Expr expEmpty = new Expr();
Expr expAdd = new Expr(ExprOprt.ADD, new AtomExpr(1.25), new AtomExpr(8.75));
Expr expSub = new Expr(ExprOprt.SUB, new AtomExpr(3.7), new AtomExpr(4.4));
Expr expMul = new Expr(ExprOprt.MUL, new AtomExpr(0.25), new AtomExpr(0.3));
Expr expDiv = new Expr(ExprOprt.DIV, new AtomExpr(0.8), new AtomExpr(2));
Expr expCplxUnnatural = new Expr(ExprOprt.DIV, expSub, expAdd);
Expr expCplxNatural = new Expr(ExprOprt.SUB, expMul, expDiv);
Assert.IsNull(expEmpty.expr0);
Assert.IsNull(expEmpty.expr1);
Assert.AreEqual(expEmpty.oprt, ExprOprt.NIL);
Assert.AreEqual(expAdd.oprt, ExprOprt.ADD);
Assert.AreEqual(expSub.oprt, ExprOprt.SUB);
Assert.AreEqual(expMul.oprt, ExprOprt.MUL);
Assert.IsNotNull(expAdd.expr0);
Assert.IsNotNull(expAdd.expr1);
Assert.IsNotNull(expSub.expr0);
Assert.IsNotNull(expSub.expr1);
Assert.IsNotNull(expMul.expr0);
Assert.IsNotNull(expMul.expr1);
Assert.IsNotNull(expDiv.expr0);
Assert.IsNotNull(expDiv.expr1);
Assert.AreEqual(expAdd.ParseValue(), 10.0, double.Epsilon);
Assert.AreEqual(expSub.ParseValue(), -0.7, double.Epsilon);
Assert.AreEqual(expMul.ParseValue(), 0.075, double.Epsilon);
Assert.AreEqual(expDiv.ParseValue(), 0.4, double.Epsilon);
Assert.AreEqual(expCplxUnnatural.ParseValue(), -0.07, double.Epsilon);
Assert.AreEqual(expCplxNatural.ParseValue(), -0.325, double.Epsilon);
Assert.AreEqual(expAdd.Priority, 0);
Assert.AreEqual(expSub.Priority, 0);
Assert.AreEqual(expMul.Priority, 1);
Assert.AreEqual(expDiv.Priority, 1);
Assert.IsTrue(expAdd.Associative);
Assert.IsFalse(expSub.Associative);
Assert.IsFalse(expDiv.Associative);
Assert.IsTrue(expMul.Associative);
Assert.IsFalse(expCplxUnnatural.Associative);
Assert.IsFalse(expCplxNatural.Associative);
Assert.IsTrue(expAdd.IsNaturalOrder);
Assert.IsTrue(expSub.IsNaturalOrder);
Assert.IsTrue(expMul.IsNaturalOrder);
Assert.IsTrue(expDiv.IsNaturalOrder);
Assert.IsFalse(expCplxUnnatural.IsNaturalOrder);
Assert.IsTrue(expCplxNatural.IsNaturalOrder);
Assert.AreEqual(expAdd.ToString(), "1.25+8.75");
Assert.AreEqual(expSub.ToString(), "3.7-4.4");
Assert.AreEqual(expMul.ToString(), "0.25*0.3");
Assert.AreEqual(expDiv.ToString(), "0.8/2");
Assert.AreEqual(expCplxNatural.ToString(), "0.25*0.3-0.8/2");
Assert.AreEqual(expCplxUnnatural.ToString(), "(3.7-4.4)/(1.25+8.75)");
}
[TestMethod()]
public void ExprUtilTest()
{
Assert.IsTrue(ExprUtil.GetOprtAssociative(ExprOprt.ADD));
Assert.IsTrue(ExprUtil.GetOprtAssociative(ExprOprt.MUL));
Assert.IsFalse(ExprUtil.GetOprtAssociative(ExprOprt.SUB));
Assert.IsFalse(ExprUtil.GetOprtAssociative(ExprOprt.DIV));
Assert.AreEqual(ExprUtil.GetOprtChar(ExprOprt.ADD), '+');
Assert.AreEqual(ExprUtil.GetOprtChar(ExprOprt.SUB), '-');
Assert.AreEqual(ExprUtil.GetOprtChar(ExprOprt.MUL), '*');
Assert.AreEqual(ExprUtil.GetOprtChar(ExprOprt.DIV), '/');
Assert.AreEqual(ExprUtil.GetOprtPriority(ExprOprt.ADD), 0);
Assert.AreEqual(ExprUtil.GetOprtPriority(ExprOprt.SUB), 0);
Assert.AreEqual(ExprUtil.GetOprtPriority(ExprOprt.MUL), 1);
Assert.AreEqual(ExprUtil.GetOprtPriority(ExprOprt.DIV), 1);
}
[TestMethod()]
public void ExprBuilderTest()
{
Random r = new Random();
ExprBuilder eBuilder = new ExprBuilder();
eBuilder.allowOprt = 0x00;
eBuilder.Allow(ExprOprt.ADD);
Assert.AreEqual(eBuilder.allowOprt, 0x01);
eBuilder.Allow(ExprOprt.DIV);
Assert.AreEqual(eBuilder.allowOprt, 0x09);
eBuilder.Disallow(ExprOprt.ADD);
Assert.AreEqual(eBuilder.allowOprt, 0x08);
eBuilder.Allow(ExprOprt.ADD);
eBuilder.Allow(ExprOprt.MUL);
eBuilder.Allow(ExprOprt.SUB);
var e = eBuilder.Generate(r);
Regex regex = new Regex(@"^(\d+)(\.\d+)?[\+|\-|\*|\/](\d+)(\.\d+)?$");
Assert.IsTrue(regex.IsMatch(e.ToString()));
eBuilder.allowDec = true;
eBuilder.maxPrecision = 5;
e = eBuilder.Generate();
Assert.IsTrue(regex.IsMatch(e.ToString()));
eBuilder.allowNeg = true;
e = eBuilder.Generate();
regex = new Regex(@"^(((\d+)(\.\d+)?)|(\(\-(\d+)(\.\d+)?\)))[\+|\-|\*|\/](((\d+)(\.\d+)?)|(\(\-(\d+)(\.\d+)?\)))$");
Assert.IsTrue(regex.IsMatch(e.ToString()));
regex = new Regex(@"^\([^\(\)]*(((?'g'\()[^\(\)]*)+((?'-g'\))[^\(\)]*)+)*(?(g)(?!))\)$");
eBuilder.maxOpnd = 5;
eBuilder.allowCplxExpr = true;
eBuilder.allowBrack = true;
e = eBuilder.Generate();
Assert.IsTrue(regex.IsMatch(String.Format("({0})",e)));
}
}
}
评估(Assessment)
程序运行结果
单元测试结果
总结
选题一时爽,结题火葬场
选题时候这道题和电梯算法那题徘徊不定,主要原因是:
- 好像都有想法该怎么做!
- 又好像完全没有想法该怎么测!!
最终考量到这道题表达的含义更加贴通近俗生易活懂(我并不经常坐电梯),于是就选了这题……
然后就是万劫不复深渊之伊始!!!
当时选择此题的时候,我和我的驾驶员都考虑了好多东西:
- 首先可以生成基本的二元四则运算
- 然后可以生成多元的四则运算
- 表达式的生成过程可以考虑使用字符串(参考曾经还有所残留的编译原理知识)
- 然后再选择性的套个括号号,美滋滋~
OK好了准备开工,然后我们惊奇的发现:
NAIVE as we are!
- 除以常数0怎么办??
- 除以了一个括号表达式,而这个括号表达式被优先计算结果也是零(隐含的0)怎么办??
- 如果不允许负数,那么一旦出现被优先计算的减法表达式的结果是个负数该怎么办??
- 如果某个位置本来不需要括号却出现了多余的括号怎么办??
……
趁着五一假期的时间,我通过远程桌面的方式引导驾驶员写代码,然后经常性的发生这样的一幕:
驾驶员与领航员看着面前的岔路口不知所措,一脸懵懵地四目相对,留下车里的X德导航的游标在地图上凌乱……
而且,作为领航员,更加令我感觉到窒息和智熄的问题是:
这玩意完全随机生成,长度不固定,括号不固定,值也不固定,我该怎么测试呢??
我总不能把所有的随机种子全丢进去(数量大致为int的长度)试一次吧??
就算我这么做,随机深度加大之后的数据我是不是也应该测呢??
是的,这次尤其令人感觉到头大的问题就是测试数据当如何选择方能达到判定覆盖或者条件覆盖
总之,上述的经历给了我一个十分惨痛的教训——
对问题的低估和不得当的规划是真的要命!
不过好在,在两个人的重整旗鼓之下,最终还是做出来了,而且效果还算不错,最终也完成了测试的过程,可喜可贺,可喜可贺……
……
……了吗??
说实话,这个程序现在仍然存在一个非常隐蔽的问题(我在做单元测试的时候发现了,是一个非常偶然性的问题),只是我还是没想到一个更好的方法去解决这个问题,尽管这个问题经过发现并不影响题目的正确生成,实属个人能力以及时间有限……
总的来说,这个作业成功地毁掉了我的五一假期同时也让我明白了很多事情,确确实实是一个非常好的教训。
之后我也会好好考虑并看看另外一道题的。
以上。