美文网首页
如何优雅的重构一段代码

如何优雅的重构一段代码

作者: jey恒 | 来源:发表于2017-12-08 14:37 被阅读45次

    首先看一段代码

    • 大概的功能是实现某个坐席领取一个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 规则中扩展所有领取业务的逻辑,很复杂的场景,此处可以做成 链式结构 调整不同类型领取的 优先级 动态配置
    • 更高级的可以做成 业务编排 当然要根据 业务场景是否需要这样

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

    相关文章

      网友评论

          本文标题:如何优雅的重构一段代码

          本文链接:https://www.haomeiwen.com/subject/xyfeuxtx.html