聊聊业务和代码重构的那些事儿

Sep 3, 2020·

3 min read

最近接了一个新需求, 是在原来的业务操作的基础之上, 需要增加两个新的流程. 如果把原来的流程看作成ABC, 新的需求则是AB1C和AB2C这种关系. 并且针对不同的客户这两种流程需要自动适配.

我的基本想法就是把原来不变的A和C, 套一个模版, 然后把B1和B2通过抽象方法来封装变化点.

想法很简单, 现实很残酷. 今天改了接近五个小时的代码, 我甚至都没有把所有的耦合点拆分完, 真的是非常难受.

我简单的总结一下问题点:

  1. 通过if else管理若干个子分支流程

    具体来说, 就是代码中出现了三个布尔值, 为了方便举例, 设为b1, b2, b3.

    public void method(){
      if (b1) {
        // do ...
        if (b2) {
          // do ...
          if (b3) {
            // do ...
          } else {
            // do ...
          }
        } else {
          // do ...
          if (b2) {
            // do ...
            if (b3) {
              // do ...
            } else {
              // do ...
            }
          }
        } else {
          // do ...
          if (b2) {
            // do ...
            if (b3) {
              // do ...
            } else {
              // do ...
            }
          } else {
            // do ...
            if (b2) {
              // do ...
              if (b3) {
                // do ...
              } else {
                // do ...
              }
            }
          }
    }
    

    当我尝试把上面这几个if else写出来的时候, 我已经晕了. 🤬 我其实觉得这种写法已经很恶心很恶心了.🤢 但更恶心的是, 每个if else又被抽成了私有化方法, 使得上面的代码变成了

    public void method(){
      // do ...
      if (b1) {
        // do ...
        doB1(b2, b3);
      } else {
        // do ...
        doNotB1(b2, b3);
      }
    }
    public void doB1(boolean b2, boolean b3){
      // do ...
      if (b2) {
        // do ...
        doB2(b3);
      } else {
        // do ...
        doNotB2(b3);
      }
    }
    public void doNotB1(boolean b2, boolean b3) {
      // do ...
      if (b2) {
        // do ...
        doB2(b3);
      } else {
        // do ...
        doNotB2(b3);
      }
    }
    public void doB2(boolean b3) {
      // do ...
      if (b3) {
        //do ...
      } else {
        //do ...
      }
    }
    public void doNotB2(boolean b3) {
      // do ...
      if (b3) {
        //do ...
      } else {
        //do ...
      }
    }
    

    而实际上的代码比我演示得还要复杂.

  2. 方法内部依赖session状态

    public class Controller {
      public String post() {
        new Service().post();
      }
    }
    public class Service {
      public void post(){
        String userId = SessionUtil.getParamter("userId");
      }
    }
    

    一个javaweb应用, 依赖于当前会话中的用户信息是🆗的, 但结合MVC以后, 需要注意的是, 你的业务逻辑层的代码, 应该是独立于会话的, 应当将你需要的东西通过参数进行传递, 而不是继续依赖于会话去减少方法的参数个数. 所以应该是在Controller这一层去获取与会话信息相关联的数据, 再传递给业务逻辑层.

    public class Controller {
      public String post() {
        new Service().post(SessionUtil.getParamter("userId"););
      }
    }
    public class Service {
      public void post(String userId){
            // do ...
      }
    }
    

    这样做还有一个好处就在于当你的方法需要增加缓存的时候, 你可以非常快速的通过AOP去完成, 而不是在你的每个方法里面去写if else来实现缓存操作.

  3. 不分析业务, 就直接堆代码

    这种做法就是简单暴力, 当接到一个需求的时候, 就是找到这个需求的代码切入点, 然后就是

    if (shouldDoSomeThingElse) {
      // do some thing else
    }
    

    写起来很爽, 维护起来很痛苦. 写的时候, 一定要结合实际代码认认真真的去考虑一下, 你新增的这个逻辑与原来的业务场景是什么样的关系.

    在我重构的这部分代码中, 有一个逻辑是这样的.

    try {
        // 检查相关数据是否正常
      // 修改属性值
      // 提交数据, 业务流程完成
      // 发送消息通知相关干系人
    } catch (Exeception e) {
      log.error("xxx", e);
    }
    

    其实我这样写的时候, 就已经把这个逻辑给分析了一遍.

    这个时候就应该把数据检查放到一个私有化方法中, 如果有不符合的则直接抛出异常. 而不是再去写一堆的if else然后返回不同的code和message

    // 错误示例
    if (StringUtils.isEmpty(p1)) {
      Map<String, String> result = new HashMap<>(2);
      result.put("code", 400);
      result.put("message", "p1为空, 提交失败");
      return map;
    }
    if (StringUtils.isEmpty(p2)) {
      Map<String, String> result = new HashMap<>(2);
      result.put("code", 400);
      result.put("message", "p2为空, 提交失败");
      return map;
    }
    if (StringUtils.isEmpty(p3)) {
      Map<String, String> result = new HashMap<>(2);
      result.put("code", 400);
      result.put("message", "p3为空, 提交失败");
      return map;
    }
    
    // 正确示例
    private void beforeDoSomeThing(String p1, String p2, String p3){
      Assert.notNull(p1, "p1为空, 提交失败");
      Assert.notNull(p2, "p2为空, 提交失败");
      Assert.notNull(p3, "p3为空, 提交失败");
    }
    

    修改属性值根据代码的行数来决定, 如果只是修改单个数据, 直接修改即可. 如果修改的值较多, 可以通过建造者模式或者是新增带有业务含义的私有化方法进行处理. 注意, 一定不要取什么setXXX() 又或者是dealXXX()这种没有任何业务含义, 需要其他开发者点进去才能知道这个方法是做啥的名字.

    提交数据这个才是真正的业务逻辑, 需要注意数据安全, 事务提交等等问问.

    最后消息发送应该是在上一个提交数据成功以后的事件通知. 这里就可以使用观察者模式了, 再根据实际的业务需要, 考虑是异步, 还是同步, 又或者是需要加入到提交数据的事务中. 良好的设计可以做到同时满足. (😊给自己挖个坑, 抽时间把这部分的设计详细展开描写一下)

作为一名2B业务的开发, 我深刻理解了没有代码是能一步到位的, 尤其是2B的业务场景, 甲方的需求千奇八怪. 在开发新需求的过程中, 要适当合理的去修改原来的设计, 而不是一昧地往上堆代码.

如果人人都能努力的做到不写乱代码, 我相信世界会变得更加美好. 一起加油.

最后的最后, 如果可以, 我们都写上单元测试吧❤️