如何优雅的重构一段代码

首先看一段代码

• 大概的功能是实现某个坐席领取一个task的功能

@Override
    public CommonResult claimOneTask(String productId,String userId,String userName, Integer isNew) {
        logger.info("productId:{},userId:{},userName:{}",productId,userId,userName);
        CommonResult result = null;
        TaskAssign ta = taskAssignService.queryTaskAssign(userId, productId, isNew);
        try {
            if(StringUtils.isBlank(userName)||StringUtils.isBlank(userId)){
                return new CommonResult(false, "当前用户为空");
            }
            if(ta != null){
                if(ta.getTasksToAssign()<1){
                    return new CommonResult(false, "待领取任务数量不足,xxxx");
                }
                Integer unCalledNum = taskDao.queryUncalledTaskCount(productId, userId, null);
                if(unCalledNum > 0){
                    return new CommonResult(false, "当前该xxx下还有等待拨打的用户，请拨打后再获取");
                }
                Long id = taskDao.queryOneTaskForUpdate(productId, isNew);
                if( null == id || id == 0){
                    return new CommonResult(false, "未领取到任务");
                }

                Task task  =  new Task();
                task.setId(id);
                task.setOwner(userId);
                task.setAssignStatus(ClaimStatus.IN_PROGRESS.getCode());
                task.setStatus(TaskClaimStatus.CLAIMED.getCode());
                task.setUpdateAt(new Date());
                task.setOwnerName(userName);
                task.setObtainedAt(new Date());
                task.setUpdateBy(userId);
                task.setChannel("outbound");
                int count = taskDao.updateTaskOwner(task);
                if(count == 0){
                    id = taskDao.queryOneTaskForUpdate(productId, isNew);
                    if( null == id || id == 0 ){
                        return new CommonResult(false, "未领取到任务");
                    }
                    task.setId(id);
                    count = taskDao.updateTaskOwner(task);
                    if(count == 0){
                        id = taskDao.queryOneTaskForUpdate(productId, isNew);
                        if( null == id || id == 0){
                            return new CommonResult(false, "未领取到任务");
                        }
                        task.setId(id);
                        count = taskDao.updateTaskOwner(task);
                    }
                }
                if(count == 0){
                    logger.info(userId + " 重试三次未领取到任务");
                    return new CommonResult(false, "领取失败，请稍后重试！");
                }
                Event evt = new Event();
                evt.setEventType("et006");
                evt.setCreateAt(new Date());
                evt.setId(IdUtils.genLongId());
                evt.setEventContent("获取任务："+id);
                evt.setCreateBy(userId);
                eventDao.saveEvent(evt);
                ta.setTasksAssigned(ta.getTasksAssigned() + 1);
                ta.setTasksToAssign(ta.getTasksToAssign() - 1);
                taskAssignService.updateTaskAssign(ta);
                CustomerVO vo =taskDao.queryCustomerByTaskId(id);
                JSONObject obj = new JSONObject();
                obj.put("idNo", vo.getIdNo());
                obj.put("customerId", vo.getCustomerId());
                obj.put("phone", vo.getPhone());
                result = new CommonResult(true, "任务领取成功",obj);
          
            }
        } catch (Exception e) {
            logger.error("获取名单时发生错误：{}",e);
            result = new CommonResult(false, "任务领取过程发生异常，领取失败");
        }
        return result;
    }

问题

• 最直观的行数多，而且try catch里面这么多if什么鬼
• 需要看半天才能看明白重试三次的逻辑，惨不忍睹
• 业务前的检查，一直new CommonResult 真的好吗
• 而且还有一个claimTheTask方法check业务一直，直接copy的代码...
• 每个方法都需要try catch？？ 不能统一处理异常？
• 事件日志记录 混在业务中间 好吗？
• 需求方现在需要 添加[平局分配领取功能，根据不同群组额度领取]，怎么加进去...
• log日志记录这样真的好吗

重构之后：

/**
   * 获取一个任务名单
   *
   * @param userId
   * @param userName
   * @param assignGroupId
   * @param isNew
   * @return
   */
  public ClaimTaskDto claimTask(String userId, String userName, String assignGroupId, Integer isNew,
      String roleCode) {

    // 分布式所控制 注解方式有问题 先不用
    if (!distributedLock.tryLock("claimTask:" + userId + ":" + assignGroupId + ":" + isNew)) {
      throw new ArgumentBizException(Code.ACCOUNT_LOCKED.getCode(), "请等三秒再尝试!");
    }

    Check.isTrue(autoOutboundModel.equals(assignGroupId), "自动外呼分配组不能获取");

//公用的check 领取的业务
    TaskAssign taskAssign = checkClaim(userId, assignGroupId, isNew);

//领取的业务
    Task task = doRetry(userId, userName, assignGroupId, isNew, taskAssign);

//领取完成后处理的业务
    ClaimTaskDto claimTaskDto =
        BeanMapper.map(getCustomerByTaskId(task.getId()), ClaimTaskDto.class);
    if (EndCodeEnum.INBOUND_CUSTOMER.getCode().equals(task.getEndCode1())) {
      claimTaskDto.setInbound(true);
      claimTaskDto.setShowMsg("有进电名单，请跟进");
    }
    return claimTaskDto;

  }

• 原来TaskService类，已经很庞大了，建议一个class是不超过2000行的，领取业务单独拆分到ClaimTaskService中
• 将领取的主体分为三部分，领取前check 重试领取的业务 和领取的后的；拆分方法的逻辑是 按照业务单元去拆分，每个方法完成什么事情

 /**
   * 领取通用检查
   *
   * @param userId
   * @param assignGroupId
   * @param isNew
   * @return
   */
  private TaskAssign checkClaim(String userId, String assignGroupId, Integer isNew) {
    TaskAssign taskAssign = taskAssignMapper.getBy(userId, assignGroupId, isNew);
    Check.notNull(taskAssign, Code.TASK_ASSIGN_NULL);
    Check.isTrue(taskAssign.getTasksLimit() - taskAssign.getTasksAssigned() < 1,
        Code.TASK_CLAIM_TODAY_LIMIT);

    // 添加一个判断
    int count = taskMapper.getTodayClaimCnt(userId, assignGroupId, isNew);
    Check.isTrue(taskAssign.getTasksLimit() - count < 1, Code.TASK_CLAIM_TODAY_LIMIT);

    int unCalledNum = taskMapper.countUncalledTask(userId);
    Check.isTrue(unCalledNum > 0, Code.TASK_HAS_NO_CALLED);
    return taskAssign;
  }

• 不要每次都是if new，if throw execption，建议封装Check类或者Assert类，guava里面的也有具体自定 [代码美观不少]

/**
   * 重试操作 可以考虑 spring retry， guava retry
   * <p>
   * 也可以简单封装下
   *
   * @param userId
   * @param userName
   * @param assignGroupId
   * @param isNew
   * @return
   */
  public Task doRetry(String userId, String userName, String assignGroupId, Integer isNew,
      TaskAssign taskAssign) {

    int retryCount = 0;

    Code code = Code.TASK_CLAIM_FAIL;
    while (retryCount < 3) {
      try {
        return doClaimTask(userId, userName, assignGroupId, isNew, taskAssign);
      } catch (RetryException e) {
        code = e.getCode();
      }
      retryCount = retryCount + 1;
    }
    log.info("[doRetry] retry three time fail userId={} isNew={} assignGroupId={}", userId, isNew,
        assignGroupId);
    throw new RetryException(code);

  }

• 定义一个RetryException 处理重试异常
• 把业务逻辑下方到 doClaimTask 方法中，跑出RetryException 就重试
• 也可以考虑引入重试组件 spring retry， guava retry
• 记录日志建议 加上方法[doRetry] =或者: 都可以

/**
   * 更新任务 可能同时很多人领取
   *
   * @param
   */
  private Task doClaimTask(String userId, String userName, String assignGroupId, Integer isNew,
      TaskAssign taskAssign) {
    InboundAssign info = inboundAssignMapper.getInboundByUserId(userId);
    Optional<Task> taskOptional = claimTaskRule(userId, userName, assignGroupId, isNew, info);
    if (!taskOptional.isPresent()) {
      throw new RetryException(Code.TASK_CLAIM_COUNT_NOT_ENOUGH);
    }

    claimTaskServiceInternal.doClaimTaskUpdate(userId, taskOptional.get(), info, taskAssign);

    eventService.logEvent(userId, "获取任务：" + taskOptional.get().getId(),
        EventTypeEnum.CLAIM_ONE_TASK);

    return taskOptional.get();
  }

• doClaimTask 才是真正领取业务开始，所有的扩展都可以在claimTaskRule 方法中
• eventService.logEvent 简单封装记录日志的 异步执行，非核心链路方法能异步的 就异步

/**
   * 获取任务规则
   *
   * @param userId
   * @param userName
   * @param assignGroupId
   * @param isNew
   * @return
   */
  private Optional<Task> claimTaskRule(String userId, String userName, String assignGroupId,
      Integer isNew, InboundAssign info) {

    // 进电任务
    Task task = inboundAssignService.getInboundTask(userId, assignGroupId,
        EndCodeEnum.INBOUND_CUSTOMER.getCode(), isNew, info);

    // 额度任务
    if (Objects.isNull(task)) {
      task = claimTaskByCredit(userId);
    }

    // 随机任务（非进电任务）
    if (Objects.isNull(task)) {
      task = claimTaskByDefault(assignGroupId, isNew);
    }

    if (Objects.isNull(task)) {
      return Optional.empty();
    }
    task.setOwner(userId);
    task.setCreateBy(userId);
    task.setAssignStatus(TaskAssignStatusEnum.IN_PROGRESS.getStatus());
    Integer status = task.getStatus();
    task.setUpdateAt(new Date());
    task.setOwnerName(userName);
    task.setUpdateBy(userId);
    task.setClaim(TaskEnum.Claim.CLAIMED.getStatus());
    return Optional.of(task);

  }

• claimTaskRule 规则中扩展所有领取业务的逻辑，很复杂的场景，此处可以做成 链式结构 调整不同类型领取的 优先级 动态配置
• 更高级的可以做成 业务编排 当然要根据 业务场景是否需要这样

其实重构一段代码很简单[多读一些基础库]，无非就是 多用组合 拆不同的类，拆方法 按最小业务单元拆，首先要建立意识，因为代码写的越容易理解就留有余地容易扩展，编码效率就很高，出错的程度也很低