The harry_potter from dpyao

Code Review Issues - 01

文件	Comments
整体代码格式问题	合理使用空行，提高代码的可读性。比如测试代码中在Given、When、Then 之间合理的使用空格以分隔不同的部分。比如产品代码中合理使用空行分隔函数中不同功能的代码块
测试的问题	测试覆盖率不够，边界值和特殊值没有涉及够，比如边界值 1本书，2本一样的书，2本不同的书等等
测试的问题	测试覆盖率不够，没有测到在new basket()后，使用addBasketItem 添加书籍再来计算总价的情况。如果认为addBasketItem不必要，那本身这个函数就没有存在的意义了。如果认为是必要的，需要测试覆盖到代码集成起来是否能正常工作
src/book.js	变量命名问题，'no' 写全名字
src/calculate-discount.js	文件或类名问题，写成名词，比如calculator 或 discount-calculator
src/calculate-discount.js	变量名问题，oldPrice 不是很合适，可以选用原价的英文单词
src/calculate-discount.js	对于basket属性的直接访问，this.basket.basketItem ，破坏了basket的封装性，建议通过basket方法而不是basket的属性来访问
src/calculate-discount.js	方法 getDiscountType 用来统计最多有基本不一样的书，对于参数 discountItem 来讲没有存在的必要，方法的返回值变成一个数字就可以了，没必要用数组。相应的方法getDiscountArray 中调用 getDiscountType 的时候直接 push 到 discountItem 中就可以了。尽量避免通过参数来传某个值然后修改这个值再传回去。
src/calculate-discount.js	方法 transferDiscountItem 中有两层嵌套，forEach 中又有一层 for，建议抽出来
src/calculate-discount.js	方法 modefiedDiscountItem 中单词 modified 拼写错误
src/calculate-discount.js	魔法数字，文件中出现了多次 8、5、4、2 之类的数字，建议用可表意的常量替换

Code Review Issues - 02

代码格式的问题举个例子：

it('should have attribute number and price', function() {
      var book = new Book(1001);

      var expect_number = 1001;

      expect(book.getNumber()).toEqual(expect_number);
});

Basket.prototype.getBookNum = function() {
  var bookNum = 0;

  this.basketItem.forEach(function(val) {
    bookNum += val.count;
  });

  return bookNum;
};

文件	Comments
整体代码格式问题	合理使用空行，提高代码的可读性。比如测试代码中在Given、When、Then 之间合理的使用空格以分隔不同的部分。比如产品代码中合理使用空行分隔函数中不同功能的代码块。
src/discount-calculator.js	常量命名问题，常量如果是多个单词组成的，中间用下划线表示。例如： `var TYPEFIVE = 5` 需要改成 `var TYPE_FIVE = 5`
src/discount-calculator.js	类名问题，类名一般采用名词。DiscountCalculate 应该改为 DiscountCalculator
src/discount-calculator.js	无用的参数，方法 getDiscountType 的参数 discountItem 没有用到，可以删除
src/discount-calculator.js	消除重复，方法 getDiscountArray 中 while 语句的判断条件其实就是方法getDiscountType 的内容，请直接替换。
spec/discount-calculator-spec.js	测试文件太长，可以采用分类成不同文件来测试。另外Line 249 和 Line 279 的测试用例容易让人迷惑，同样的数据得到了不同的结果。这个在测试描述中又没有说明区别。
src/strategy.js	方法 getStrategy 在产品代码中没有使用，只在测试文件中使用。能用的地方可能是在方法 `DiscountCalculate.prototype.getDiscountPrice` 中。
src/discount-calculator.js	方法 calculateDiscountPrice 中对特殊组合‘5-3’进行了处理，可以专门提出函数来做
src/discount-calculator.js	命名，getDiscountType、getDiscountArray、getDiscountObjItem 从名字来看有点晕
src/discount-calculator.js	方法 calculateDiscountPrice 中满足什么样的条件才需要 modifyDiscountItem。方法本身可以简化，比如将条件语句转换成 `var min = Math.min(typeFiveCount, typeThreeCount); discountObjItem = this.modifyDiscountItem(discountObjItem, min);` 在方法 modifyDiscountItem 中也可以简化，只把5-3组合的个数改成4-4的个数，原来5-3组合个数分别减少min个就好了。

dpyao / harry_potter Goto Github PK

harry_potter's People

Watchers

harry_potter's Issues

Code Review Issues - 01

Code Review Issues - 02

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent