一、需求开发修改代码
一次需求开发时碰到如下所示方法代码：
private OrderShoudSettlementAmount getOrderShoudSettlementAmount(OrderDTO orderMain, List<SettlementDetail> details) {
OrderShoudSettlementAmount settlementAmount = new OrderShoudSettlementAmount();
// 应结金额=33021-33002-32003+32001-31001
// 货款佣金=33005+33002+32003+31001
long feeMoney33021 = 0;
long feeMoney33002 = 0;
long feeMoney32003 = 0;
long feeMoney32001 = 0;
long feeMoney31001 = 0;
long feeMoney33005 = 0;
for (SettlementDetail settlementDetail : details) {
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_FREIGHT_ZS_NOSETTLE.getVal())) {
feeMoney33021 += settlementDetail.getOassMoney();
}
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_FREIGHT_YJ_ZS_NOSETTLE.getVal())) {
feeMoney33002 += settlementDetail.getOassMoney();
}
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_GOODS_YJ_WJTZ.getVal())) {
feeMoney32003 += settlementDetail.getOassMoney();
}
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_GOODS_NOSETTLE.getVal())) {
feeMoney32001 += settlementDetail.getOassMoney();
}
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_BDYJ_YJ_NOSETTLE.getVal())) {
feeMoney31001 += settlementDetail.getOassMoney();
}
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_YFYJ_ZS_XSG.getVal())) {
feeMoney33005 += settlementDetail.getOassMoney();
}
}
long settlementMoney = feeMoney33021 - feeMoney33002 - feeMoney32003 + feeMoney32001 - feeMoney31001;
long goodCommissionMoney = feeMoney33005 + feeMoney33002 + feeMoney32003 + feeMoney31001;
settlementAmount.setSettlementAmount(settlementMoney);
settlementAmount.setGoodsCommission(goodCommissionMoney);
settlementAmount.setOrderId(orderMain.getOrderId());
settlementAmount.setOrgCode(orderMain.getOrgCode());
settlementAmount.setStationNo(String.valueOf(orderMain.getDeliveryStationNo()));
settlementAmount.setBillTime(new Date());
settlementAmount.setRetSuccess(false);
return settlementAmount;
}
该方法逻辑比较简单，就是组装OrderShoudSettlementAmount对象。其中需要计算2个金额，分别是settlementMoney和goodCommissionMoney。
本次需求新增了费项，需要修改该方法。代码修改后如下所示：
private OrderShoudSettlementAmount getOrderShoudSettlementAmount(OrderDTO orderMain, List<SettlementDetail> details) {
OrderShoudSettlementAmount settlementAmount = new OrderShoudSettlementAmount();
// 应结金额=33021-33002-32003+32001-31001+34012-34013
// 货款佣金=33005+33002+32003+31001+34013
long feeMoney33021 = 0;
long feeMoney33002 = 0;
long feeMoney32003 = 0;
long feeMoney32001 = 0;
long feeMoney31001 = 0;
long feeMoney33005 = 0;
// 本次需求新增费项
long feeMoney34012 = 0;
// 本次需求新增费项
long feeMoney34013 = 0;
for (SettlementDetail settlementDetail : details) {
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_FREIGHT_ZS_NOSETTLE.getVal())) {
feeMoney33021 += settlementDetail.getOassMoney();
}
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_FREIGHT_YJ_ZS_NOSETTLE.getVal())) {
feeMoney33002 += settlementDetail.getOassMoney();
}
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_GOODS_YJ_WJTZ.getVal())) {
feeMoney32003 += settlementDetail.getOassMoney();
}
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_GOODS_NOSETTLE.getVal())) {
feeMoney32001 += settlementDetail.getOassMoney();
}
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_BDYJ_YJ_NOSETTLE.getVal())) {
feeMoney31001 += settlementDetail.getOassMoney();
}
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_YFYJ_ZS_XSG.getVal())) {
feeMoney33005 += settlementDetail.getOassMoney();
}
// 本次需求新增费项
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_CHF_XSG_NOSETTLE.getVal())) {
feeMoney34012 += settlementDetail.getOassMoney();
}
// 本次需求新增费项
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_CHF_YJ_XSG.getVal())) {
feeMoney34013 += settlementDetail.getOassMoney();
}
}
// 本次需求新增费项追加计算 + feeMoney34012 - feeMoney34013
long settlementMoney = feeMoney33021 - feeMoney33002 - feeMoney32003 + feeMoney32001 - feeMoney31001 + feeMoney34012 - feeMoney34013;
// 本次需求新增费项追加计算 + feeMoney34013
long goodCommissionMoney = feeMoney33005 + feeMoney33002 + feeMoney32003 + feeMoney31001 + feeMoney34013;
settlementAmount.setSettlementAmount(settlementMoney);
settlementAmount.setGoodsCommission(goodCommissionMoney);
settlementAmount.setOrderId(orderMain.getOrderId());
settlementAmount.setOrgCode(orderMain.getOrgCode());
settlementAmount.setStationNo(String.valueOf(orderMain.getDeliveryStationNo()));
settlementAmount.setBillTime(new Date());
settlementAmount.setRetSuccess(false);
return settlementAmount;
}
二、嗅出代码的坏味道
Martin Fowler在《重构：改善既有代码的设计》一书中列出了22种代码的坏味道：
参照这22种代码的坏味道，我在以上方法代码中嗅出了2种代码的坏味道：
坏味道1：Duplicated Code（重复的代码）
for循环中对每种费项的累加操作是重复代码，而且每次新增费项，还得不断增加该重复操作。
for (SettlementDetail settlementDetail : details) {
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_FREIGHT_ZS_NOSETTLE.getVal())) {
feeMoney33021 += settlementDetail.getOassMoney();
}
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_FREIGHT_YJ_ZS_NOSETTLE.getVal())) {
feeMoney33002 += settlementDetail.getOassMoney();
}
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_GOODS_YJ_WJTZ.getVal())) {
feeMoney32003 += settlementDetail.getOassMoney();
}
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_GOODS_NOSETTLE.getVal())) {
feeMoney32001 += settlementDetail.getOassMoney();
}
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_BDYJ_YJ_NOSETTLE.getVal())) {
feeMoney31001 += settlementDetail.getOassMoney();
}
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_YFYJ_ZS_XSG.getVal())) {
feeMoney33005 += settlementDetail.getOassMoney();
}
// 本次需求新增费项
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_CHF_XSG_NOSETTLE.getVal())) {
feeMoney34012 += settlementDetail.getOassMoney();
}
// 本次需求新增费项
if (settlementDetail.getExpenseType().equals(FeeInfoEnum.FEE_INFO_CHF_YJ_XSG.getVal())) {
feeMoney34013 += settlementDetail.getOassMoney();
}
}
坏味道2：Divergent Change（发散式变化）
Martin Fowler在书中对该坏味道的部分解释如下：
现在该方法代码因为新需求开发，修改多处。
其实，除了以上2种代码的坏味道之外，该方法代码最大的问题是面向过程式编码而不是面向对象式的。
为什么这么说呢？
前面提到过该方法的主要作用是组装OrderShoudSettlementAmount对象，那么其逻辑就应该主要体现“组装”，而不是计算金额。计算金额相关逻辑应该抽离到单独的类中，这样既符合面向对象编程思想，也能够消除坏味道2。
三、重构代码
针对前面嗅出的代码坏味道，果断进行重构。重构之后代码如下所示：
private OrderShoudSettlementAmount getOrderShoudSettlementAmount(OrderDTO orderMain, List<SettlementDetail> details) {
OrderShoudSettlementAmount settlementAmount = new OrderShoudSettlementAmount();
Map<Integer, Long> expenseTypeToFeeMoneyMap = Maps.newHashMap();
for (SettlementDetail settlementDetail : details) {
long feeMoney = Optional.ofNullable(expenseTypeToFeeMoneyMap.get(settlementDetail.getExpenseType())).orElse(0L);
feeMoney += Optional.ofNullable(settlementDetail.getOassMoney()).orElse(0L);
expenseTypeToFeeMoneyMap.put(settlementDetail.getExpenseType(), feeMoney);
}
long settlementMoney = SettlementMoneyCalcFeeInfoEnum.calcSettlementMoney(expenseTypeToFeeMoneyMap);
long goodCommissionMoney = GoodCommissionMoneyCalcFeeInfoEnum.calcGoodCommissionMoney(expenseTypeToFeeMoneyMap);
settlementAmount.setSettlementAmount(settlementMoney);
settlementAmount.setGoodsCommission(goodCommissionMoney);
settlementAmount.setOrderId(orderMain.getOrderId());
settlementAmount.setOrgCode(orderMain.getOrgCode());
settlementAmount.setStationNo(String.valueOf(orderMain.getDeliveryStationNo()));
settlementAmount.setBillTime(new Date());
settlementAmount.setRetSuccess(false);
return settlementAmount;
}
enum SettlementMoneyCalcFeeInfoEnum {
/**计算项*/
FEE_33021(FeeInfoEnum.FEE_INFO_FREIGHT_ZS_NOSETTLE, "+"),
FEE_33002(FeeInfoEnum.FEE_INFO_FREIGHT_YJ_ZS_NOSETTLE, "-"),
FEE_32003(FeeInfoEnum.FEE_INFO_GOODS_YJ_WJTZ, "-"),
FEE_32001(FeeInfoEnum.FEE_INFO_GOODS_NOSETTLE, "+"),
FEE_31001(FeeInfoEnum.FEE_INFO_BDYJ_YJ_NOSETTLE, "-"),
FEE_34012(FeeInfoEnum.FEE_INFO_CHF_XSG_NOSETTLE, "+"),
FEE_34013(FeeInfoEnum.FEE_INFO_CHF_YJ_XSG, "-");
private final FeeInfoEnum feeInfoEnum;
private final String symbol;
SettlementMoneyCalcFeeInfoEnum(FeeInfoEnum feeInfoEnum, String symbol) {
this.feeInfoEnum = feeInfoEnum;
this.symbol = symbol;
}
public static long calcSettlementMoney(Map<Integer, Long> expenseTypeToFeeMoneyMap) {
// 应结金额=33021-33002-32003+32001-31001+34012-34013
long settlementMoney = 0L;
for (SettlementMoneyCalcFeeInfoEnum calcFeeInfoEnum : SettlementMoneyCalcFeeInfoEnum.values()) {
if ("+".equals(calcFeeInfoEnum.symbol)) {
settlementMoney += Optional
.ofNullable(expenseTypeToFeeMoneyMap.get(calcFeeInfoEnum.feeInfoEnum.getVal()))
.orElse(0L);
}
if ("-".equals(calcFeeInfoEnum.symbol)) {
settlementMoney -= Optional
.ofNullable(expenseTypeToFeeMoneyMap.get(calcFeeInfoEnum.feeInfoEnum.getVal()))
.orElse(0L);
}
}
return settlementMoney;
}
}
enum GoodCommissionMoneyCalcFeeInfoEnum {
/**计算项*/
FEE_33005(FeeInfoEnum.FEE_INFO_YFYJ_ZS_XSG),
FEE_33002(FeeInfoEnum.FEE_INFO_FREIGHT_YJ_ZS_NOSETTLE),
FEE_32003(FeeInfoEnum.FEE_INFO_GOODS_YJ_WJTZ),
FEE_31001(FeeInfoEnum.FEE_INFO_BDYJ_YJ_NOSETTLE),
FEE_34013(FeeInfoEnum.FEE_INFO_CHF_YJ_XSG);
private final FeeInfoEnum feeInfoEnum;
GoodCommissionMoneyCalcFeeInfoEnum(FeeInfoEnum feeInfoEnum) {
this.feeInfoEnum = feeInfoEnum;
}
public static long calcGoodCommissionMoney(Map<Integer, Long> expenseTypeToFeeMoneyMap) {
// 货款佣金=33005+33002+32003+31001+34013
long goodCommissionMoney = 0L;
for (GoodCommissionMoneyCalcFeeInfoEnum calcFeeInfoEnum : GoodCommissionMoneyCalcFeeInfoEnum.values()) {
goodCommissionMoney += Optional
.ofNullable(expenseTypeToFeeMoneyMap.get(calcFeeInfoEnum.feeInfoEnum.getVal()))
.orElse(0L);
}
return goodCommissionMoney;
}
}
四、总结
以上重构的方法代码比较简单，有些人可能会觉得不重构也挺好的，代码可读性也不差，每次修改也就肉眼可见的几个地方，没必要在这上面花费时间。
如果你有以上想法，不妨了解下软件工程中的“破窗效应”：
同时看看Martin Fowler在《重构：改善既有代码的设计》一书中对重构的部分解释：
重构不仅能够提高代码质量，让代码优雅起来，同时也能让我们学以致用。我们所学的设计思想、原则、模式等理论知识，往往在重构中能够真正实践。