如何优雅的重构一段代码
2017-12-08 本文已影响45人
jey恒
首先看一段代码
- 大概的功能是实现某个坐席领取一个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 规则中扩展所有领取业务的逻辑，很复杂的场景，此处可以做成 链式结构 调整不同类型领取的 优先级 动态配置
- 更高级的可以做成 业务编排 当然要根据 业务场景是否需要这样
其实重构一段代码很简单[多读一些基础库]，无非就是 多用组合 拆不同的类，拆方法 按最小业务单元拆，首先要建立意识，因为代码写的越容易理解就留有余地容易扩展，编码效率就很高，出错的程度也很低
