重构这段烂代码，差点把我整凌乱了...

2023-11-30 09:00 由 buguge 发表于 #后端开发

🍀注重实效，不要靠巧合编程。
🍀在构造一个对象的过程中，下文尽量不要依据对象已经set了值的field做判断然后继续给对象其他field赋值，而应该基于原有对象的field去判断。

先看这段代码，烂不烂，你可以品一下，多半味道不怎么好。

 1 try {
 2     final ResponseDTO responseDTO = bindCardService.bindCard(bindCardReqDTO);
 3 
 4     if (ObjectUtil.isNotNull(responseDTO)) {
 5         // 变更流水、绑卡记录状态->待认证
 6         bankCardFlow.setRemark(responseDTO.getReturnMsg());
 7         bankCardFlow.setStatus(StatusEnum.UNAUTH.getCode());
 8         bankCardFlow.setUpdateTime(updateTime);
 9 
10         bankCard.setStatus(StatusEnum.UNAUTH.getCode());
11         bankCard.setUpdateTime(updateTime);
12 
13         payMerchantBankCardManager.updateById(bankCard);
14         payMerchantBankCardFlowManager.updateById(bankCardFlow);
15     }
16     return Result.success();
17 } catch (Exception e) {
18     log.error("亿联-绑卡失败-异常：", e);
19     if (e instanceof YiLianBizException) {
20         bankCardFlow.setRemark(e.getMessage());
21     } else {
22         bankCardFlow.setRemark("出现异常-绑卡失败");
23     }
24     bankCardFlow.setUpdateTime(updateTime);
25     bankCard.setUpdateTime(updateTime);
26     if (bankCardFlow.getRemark().contains(YiLianConstant.TRADE_RESULT_UNKNOWN)) {
27         bankCardFlow.setStatus(StatusEnum.UNAUTH.getCode());
28         bankCard.setStatus(StatusEnum.UNAUTH.getCode());
29         payMerchantBankCardManager.updateById(bankCard);
30         payMerchantBankCardFlowManager.updateById(bankCardFlow);
31         return Result.success();
32     } else {
33         bankCardFlow.setStatus(StatusEnum.FAIL.getCode());
34         bankCard.setStatus(StatusEnum.FAIL.getCode());
35         payMerchantBankCardManager.updateById(bankCard);
36         payMerchantBankCardFlowManager.updateById(bankCardFlow);
37         return Result.err("亿联-绑卡失败,请重新绑卡");
38     }
39 }

在不改变业务逻辑的基础上，我重构这段代码，如下。将单表update操作做了封装，这里直接调用封装好的方法，同时增加了状态锁控制。不过，一个点竟把我绕进去了。见代码中所加的TODO:“ 这里为什么判断bankCardFlow.getRemark()？ ---逻辑被我改坏了？”。

隔了一天，我回头再review这段代码，跟此前版本比较，终于看明白了。聪明的你，看出来破绽了吧？ -------我差点犯了个错误呀，赶紧修正，把 bankCardFlow.getRemark().contains(YiLianConstant.TRADE_RESULT_UNKNOWN) 中的 bankCardFlow.getRemark() 改成 msg 变量。

 1 try {
 2     final ResponseDTO responseDTO = bindCardService.bindCard(bindCardReqDTO);
 3 
 4     if (ObjectUtil.isNotNull(responseDTO)) {
 5         // 变更流水、绑卡记录状态->待认证
 6         payMerchantBankCardManager.updateStateTo(bankCard, StatusEnum.UNAUTH, StatusEnum.INIT);
 7         payMerchantBankCardFlowManager.updateStateTo(bankCardFlow, StatusEnum.UNAUTH, StatusEnum.INIT, responseDTO.getReturnMsg());
 8     }
 9     return Result.success();
10 } catch (Exception e) {
11     String msg;
12     if (e instanceof YiLianBizException) {
13         log.error("亿联-绑卡失败：{}", e.getMessage());
14         msg = e.getMessage();
15     } else {
16         log.error("亿联-绑卡失败-异常：", e);
17         msg = "出现异常-绑卡失败" + ExceptionUtils.getMessage(e);
18     }
19     //TODO: 这里为什么判断bankCardFlow.getRemark()？ ---逻辑被我改坏了？
20     if (bankCardFlow.getRemark().contains(YiLianConstant.TRADE_RESULT_UNKNOWN)) {
21         payMerchantBankCardManager.updateStateTo(bankCard, StatusEnum.UNAUTH, StatusEnum.INIT);
22         payMerchantBankCardFlowManager.updateStateTo(bankCardFlow, StatusEnum.UNAUTH, StatusEnum.INIT, msg);
23         return Result.success();
24     } else {
25         payMerchantBankCardManager.updateStateTo(bankCard, StatusEnum.FAIL, StatusEnum.INIT);
26         payMerchantBankCardFlowManager.updateStateTo(bankCardFlow, StatusEnum.FAIL, StatusEnum.INIT, msg);
27         return Result.err("亿联-绑卡失败,请重新绑卡");
28     }
29 }

通常，在构造一个对象的过程中，下文尽量不要依据对象已经set了值的field做判断然后继续给对象其他field赋值，而应该基于原有对象的field去判断。
【反例】下面if判断中使用了order#payType：

Order order = new Order();
...
order.setPayType(payRequest.getPayType());
...
if ("BANKCARD".contentEquals(order.getPayType())) {
    order.setBankCardNo(payRequest.getBankCardNo();
    order.setBankName(payRequest.getBankName());
} else if ("ALIPAY".contentEquals(order.getPayType())) {
    order.setAliAccount(payRequest.getAliUserId());
    ...

【正例】把if判断条件改为既定对象（入参或读库或调用其他接口得到的对象）payRequest#payType：

Order order = new Order();
...
order.setPayType(payRequest.getPayType());
...
if ("BANKCARD".contentEquals(payRequest.getPayType())) {
    order.setBankCardNo(payRequest.getBankCardNo();
    order.setBankName(payRequest.getBankName());
} else if ("ALIPAY".contentEquals(payRequest.getPayType())) {
    order.setAliAccount(payRequest.getAliUserId());
    ...

有同学会反问，这有什么区别呢？

有的。

当Order的属性较多时，构造Order对象的代码行会比较多。如果按照【反例】做的话，代码维护过程中一旦出现先get后set的情况，将会导致程序bug。——不可能出现这样的事情？！别说不可能，只能说你还没有遇到过。

细微处见成色！我们在日常编码中，要注重细节，注重实效，不要靠巧合编程。拿本案来说，就是要按【正例】做，不给bug留空子。

热门相关：重生之神级败家子农家小地主重生之神级败家子战国明月我家老公超宠哒