once do, do it well

CASE1：http工具类调用

昨天出现一个生产问题。我们的channel系统代码里，调用其中一个三方服务商的http接口时未设置超时时间。碰巧昨天出现一笔http请求持续数小时始终无响应，加之程序是单线程处理交易请求，就出现因为线程一直处于RUNNABLE状态而导致系统生产能力严重下降。

现在说这个结论很easy，而当时在排查这个问题时很是费了几番周折。

那么，解决这个问题，自然是为这个服务商的http请求设置合理的超时时间。

组内的小伙很快fix了这段代码，为方法里的http请求设置了connectTimeout和socketTimeout。

 

发现问题，上来就解决，往往是低效的方式。

为什么这么说呢？

曾经我们系统化地调整过channel里的对外http连接的超时时间。而怎么单单遗漏了这个服务商呢？原来，查看代码才发现，是这个服务商并没有依赖我们的公共的http util类，而是单独写的http post方法，藏匿得比较深。

consequently，once you do,do it well。通过review代码后，我们改成了让这个服务商也调用公共的http util来实现http通信，同时删掉当前class里的那个post方法。

 

CASE2：躲避equals的坑

channel系统里，调用服务商下发结果查询的链路关系如下。

其中，各个③实现了相同的interface LevyAPI。LevyPayRouteFacade#queryPayResult 与 LevyAPI#queryPayResult 的返回值是LevyPayQueryResultVO。显然，既然是下发结果查询，LevyPayQueryResultVO里有一个payStatus字段，payStatus字段的类型是String。

系统里有为下发结果状态定义的枚举 LevyPaymentStatusEnum。

注意到payStatus的类型是String。各个LevyAPI实现类里，在调用服务商api得到结果后要转成我们的payStatus，同时，①LevyPaymentService#queryPayResult里得到查询结果后要对不同的payStatus做相关逻辑，字符串类型的payStatus为这些代码的可读性和可扩展性带来了诸多不便。

有鉴于此，我们重构，将payStatus的类型改为枚举类型LevyPaymentStatusEnum。

终于到可以开始说正题的时候了。重构完成后呢，build没问题。而在部署到环境后，发现了一个问题，这个问题由equals引起，看下面代码。

    // 服务商返回 成功 状态
    if (LevyPaymentFlowStatusEnum.PAY_SUCCESS.getCode().equals(queryResultVO.getStatus())){
        update = levyPaymentManager.updateQueryResult(queryResultVO, entity, LevyPaymentFlowStatusEnum.PAY_SUCCESS);
    }
    // 服务商返回 失败 状态
    else if (LevyPaymentFlowStatusEnum.PAY_FAILED.getCode().equals(queryResultVO.getStatus())){
        update = levyPaymentManager.updateQueryResult(queryResultVO, entity, LevyPaymentFlowStatusEnum.PAY_FAILED);
    }
    // 服务商返回 未处理/处理中/结算中/其他 状态时，要改成付款中
    else{
        update = levyPaymentManager.updateQueryResult(queryResultVO, entity, LevyPaymentFlowStatusEnum.PAY_DEALING);
    }

显然，将payStatus由String改为LevyPaymentStatusEnum，不会对这里的equals方法有丝毫影响，IDE也不会提示你改这里。开发人员注意不到的话，就为漏洞埋下伏笔（什么漏洞？你懂的）。那么，发现这个问题后，这段代码被修正为下面的样子。

    if (LevyPaymentFlowStatusEnum.PAY_SUCCESS.equals(queryResultVO.getStatus())){
        ...
    } else if (LevyPaymentFlowStatusEnum.PAY_FAILED.equals(queryResultVO.getStatus())){
        ...
    } else{
        ...
    }

 

Well, what I want to say is, once you do, do it well.
既然equals有坑，那么，我们应该再进行调优，去掉equals，直接使用==来比较枚举。

    if (LevyPaymentFlowStatusEnum.PAY_SUCCESS == queryResultVO.getStatus()){
        ...
    } else if (LevyPaymentFlowStatusEnum.PAY_FAILED == queryResultVO.getStatus()){
        ...
    } else{
        ...
    }

 

What I want to say is, please do not use the equals method unless you have no other choice. In many cases, use String#contentEquals instead of String#equals is a  good practise.