聊聊那些年遇到过的奇葩代码-编程学习网

奇葩代码大赏

命名没有业务语义

public void handleTask(Long taskId, Intger status){
    TaskModel taskModel = taskDomainService.getTaskById(taskId);
    Assert.notNull(taskModel);
    taskModel.setStatus(status);
    taskDomainService.preserveTask(taskModel);
}

可能乍一看这段代码其实没啥大问题，但是如果要知道这段代码到底是干嘛的可能你一下子反应不过来，需要好好看看代码逻辑才知道。通过查看代码我们知道此处的代码业务语义是变更任务状态，但是实际的方法名称是handleTask，命名明显过于宽泛了，不能精确表达实际的业务语义。

那么为什么要把代码撸一遍才能明确方法的含义呢？归根到底就是方法命名不够准确，不能完全表达这段代码所对应的业务语义。那为什么我们经常不能很准确的进行类或者方法的命名呢？我想最根本的原因还是码代码的同学没能够精准把握这段代码的业务语义，因此在起名字的时候要么过于宽泛，要么词不达意。

因此无论是类命名或者方法命名都要能够明确的表达业务语义，只有这样无论是一段时间自己回过头来看或者其他维护者来看代码都能够通过看命名就可以明确代码蕴含的业务逻辑。

单个方法过长

特别是在一些老项目中，我们经常会遇到一个方法里面能塞进去几百行代码。一般造成这种单个方法代码过长的原因无非有两个，一个是用过程化的思维编写代码，想到哪些业务步骤都统统写在一个方法中；另一个就是后来的维护者需要增加新的功能，一看代码这么长也不敢瞎改只能在长方法中继续码代码，造成方法原来越长难以维护。

无论是从后期代码可维护性还是从SRP设计原则来说，单个方法中代码行数最好不要超过100行，否则带来的后果就是各种业务逻辑糅合在一起，不仅后期维护代码的同学不容易理解其中包含的业务语义，而且如果功能变化修改起来也比较费劲。

public void shelveFreshGoods(){
    //检查货品
    //几十行代码（检查重量、检查新鲜度等等）
    
    //货品摆渡
    //几十行代码（生成货品编号、装载等等）
    
    //上架
    //几十行代码（货品打标、绑定库存等等）
    ...
    
}

如上架鲜品的逻辑，可以看的出来在上架生鲜产品的时候会经历货品检查、货品摆渡、货品上架等多个个步骤，但是在这个shelveFreshGoods方法中将这些业务步骤走杂糅在了一起，如果我们想修改或者增加业务逻辑的时候就需要在这个方法中只能在这个长方法中进行修改，可能会导致方法越来越长。而如果通过拆分的方式进行业务子过程划分，也就是说将上述的几个步骤都封装成方法。那么修改某业务逻辑可直接在对应拆出来的步骤中进行，这样修改的范围就缩小了，另外业务逻辑看上去一目了然。

public void shelveFreshGoods(){
 //检查货品
  check();
  //货品摆渡
  transfer();
 //上架
  shelve();  
}

业务数据循环插入

在进行业务代码开发的时候，批量进行业务数据插入是非常常见的CRUD基操。但是有的同学在写批量插入接口的时候会这么写，通过for循环或者stream来进行循环数据写入。这样的写法会平白增加服务与数据库的交互次数，占用不必要的数据库连接，很容易遇到性能问题。如果一次性插入的数据不多的话（几条数据）倒也影响不大，但是如果数据量多起来的话必定会成为性能瓶颈。

for(TaskPO taskPO : taskPOList) {
    saveTask(taskPO);
}

很明显可以看得出来，原先的写法需要与数据库进行多次交互。而优化后的写法只需要和数据库交互一次。实际上我们可以在mapper文件中进行批量插入进行优化，这样实际上通过批量插入的sql语句，从而实现服务与数据库只交互一次就完成数据的批量保存。

<insert id="batchSaveTask" parameterType="java.util.List">  
  insert into task   
  (c_id,c_name,c_type,c_content,c_operator,i_container_type,c_warehouse_type) 
  values   
  <foreach collection="taskList"  item="item" open="(" close=")" separator=",">  
    (#{item.id},#{item.type},#{item.content},#{item.operator},#{item.containerType},#{item.warehouseType}) 
  foreach>  
insert>

先查数据再更新数据库

在进行业务代码编写的时候，经常会碰到这样的场景，如果数据库中有数据则进行更新，如果没有数据则直接插入。我们来看看下面这种写法，先从数据库中查询数据，如果存在则进行更新，如果不存在则进行数据插入，有两次数据库交互操作。

Task task = taskBizService.queryTaskByName();
if(Objects.isNull(task)) {
  taskBizService.saveTask(); //省略参数
}
taskBizService.updateTask(); //省略参数

实际上可以直接通过数据库的sql进行控制，存在数据则进行更新，不存在则插入，这样可以避免和数据库的多次交互。

insert into task(c_id,c_name,c_type,c_content,c_operator,i_container_type,c_warehouse_type) values (#{name},#{type},#{content},#{operator},#{containerType},#{warehouseType}) on conflicct(c_name) do update set c_cnotallow=#{content}

业务依赖技术细节

我们先来看下Robert C. Martin提出来的依赖倒置原则怎么描述的：

High-level modules should not depend on low-level modules. Both should depend on abstractions.

高层模块不应该依赖于低层模块，二者都应该依赖于抽象。

Robert C. Martin

Abstractions should not depend on details. Details (concrete implementations) should depend on abstractions.

抽象不应该依赖于细节，细节应该依赖于抽象。

Robert C. Martin

这两句话听上去有点不明觉厉，不如我们结合下具体的业务场景更好理解一点。假设在一个监控告警平台中，如果线上平台出现了问题，比如调用订单生成接口失败，无法生成订单。平台检测到这样的异常之后需要通知研发同学进行问题排查定位。此时监控告警平台会将告警信息发送到钉钉群中进行通知。因此我们需要一个发送钉钉消息的接口，如下所示。

public boolean sendDingTalk(Message message){
        ...
    }

看上去代码是没什么问题的，有了告警就调用发送钉钉消息的接口方法。但是实际上这样的写法违反了依赖倒置的设计原则。为什么这么说，试想一下如果哪天公司决定不用钉钉接收告警信息，改用企业微信了或者是自己公司的通讯软件。那么此处的sendDingTalk必定是要进行修改的，因为我们的告警通知业务依赖了具体的发送消息通知的实现细节，这明显是不合理的。

因此此处比较好的做法是，定义一个notifyMessage的接口，具体的实现细节上层不必关心，无论是通过钉钉通知还是企业微信通知也好，只要实现这个通知的接口就OK了。即便后期进行切换，原来的业务逻辑并不需要进行修改，只要修改具体通知接口的实现就可以了。

public interface NotifyMessage {
    boolean notifyMessage(Message message);

}

public class DingTalk implements NotifyMessage {
 
  @Override
  public boolean notifyMessage(Message message){
       ...
    }
}

public class WeChat implements NotifyMessage {
  
   @Override
   public boolean notifyMessage(Message message){
       ...
    }
}

长SQL

程序猿接手项目的时候，最怕遇到的就是项目中那些动不动上百行的长SQL。这些长SQL中有的存在各种嵌套查询，甚至包含了三四层子查询；有的包含了四五个left join，inner join连接了五六张表，这些长SQL一个电脑屏幕都装不下，仿佛装不下的还有写这个长SQL的同学的“才华”。更无语的是如果写这个SQL的同学已经离职了，你想问下大致的查询逻辑都没人可以问，即便是没有离职，写的人过了一段时间后再看这段SQL估计也挺费劲。

可能有的同学会说我也不想写长SQL啊，奈何数据分散在各个表中，业务逻辑也比较复杂，所以只能各种join各种子查询，不知不觉就写了长SQL。但是实际上长SQL并不能解决上述数据分散业务复杂的问题，反而带来了后期维护差等各种问题，长SQL表面上看是一个数据库操作，但是在数据库引擎层面还是将长SQL分成了多个子操作，各个子操作完成后再将结果数据进行统一返回。

那么如何避免写出来这种维护性很差的长SQL呢？对于一些查询场景比较多的长SQL可以尝试使用大宽表来承载需要展示的各个字段数据，这样页面查询的时候直接在大宽表上进行查询，而不必再组合各个业务数据进行查询，或者将又有的长SQL拆分成多个视图以及存储过程来简化SQL的复杂性。

接口参数过多

这个问题在实际项目开发中经常遇到，当你需要调一个别人封装好的接口的时候，对方突然丢过来一个方法包含了七八个参数。我想当时你的心情应该是想对他深深说一句真是栓Q你了。其实对于一个方法的参数来说，这里建议参数个数还是最多不要超过5个。

Integer preserveTask(String taskId, 
             String taskName, 
             String taskType, 
             String taskContent,
             String operator,
             Integer containerType,
             String warehouseType);

实际上我们可以用模型对象来进行参数封装，这样可以避免方法中参数个数过多导致后期维护困难。因为随着业务的发展，有可能会出现修改接口能力来满足新的需求，但是这个时候如果动接口参数的话，那么对应的接口以及实现类都需要修改，万一有其他地方调用这个接口，那么修改的地方就会更多，很明显这不符合OCP设计原则。因此这个时候如果使用的是一个对象作为方法的参数，那么无论是增加或者减少参数都只需要修改参数对象，并不需要修改对应方法的接口参数，这样接口的扩展性会更加强一点。因此我们在写代码的时候不能光着眼于当下，还要考虑对应需求发生变化的时候，我的代码怎么才能适应这种变化做到最小化修改，后期无论是自己维护还是别人的同学维护都会更加方便一点。

Integer preserveTask(TaskDO taskDO);

重复代码

之前专门写过关于如何消除系统重复的代码的文章，具体可以参见如下：

如何优雅的消除系统重复代码

常见代码优化写法

尽量复用工具函数

集合判断

日常开发的时候我们经常遇到关于数据集合非空判断的逻辑，常见的写法如下，虽然没什么问题但是看起来非常不顺溜，简单来说就是不够直接，一眼望过去还得反应一下。

if(null != taskList && !taskList.isEmpty()) {
    //业务逻辑
}

但是通过使用封装好的工具类直接进行判断，所看即所得，清楚明白表达集合检查逻辑。

if(CollectionUtils.isNotEmpty(taskList)) {
    //业务逻辑
}

Boolean转换

在一些场景下我们需要将Boolean值转化为1或者0，因此常见如下代码：

if(switcher) {
    return 1;
} else {
 return 0;
}

实际上可以借助于工具方法简化为如下代码：

return BooleanUtils.toInteger(switcher);

lambda表达式简化集合

集合最常见的场景就是进行数据过滤，筛选出符合条件的对象，代码如下：

List<Student> oldStudents = new ArrayList();
for(Student student: studentList) {
 if(student.getAge() > 18) {
        oldStudents.add(student);
    }
}

实际上我们可以利用lambda表达式进行代码简化：

List<Student> oldStudents = studentList.stream()
                            .filter(item -> item.getAge() > 18)
                            .collect(Collectors.toList());

Optional减少if判断

假设我们要获取任务的名称，如果没有则返回unDefined，传统的写法可能是这样，包含了多个if判断，看上去有点啰里啰唆不够简洁。

public String getTaskName(Task task){
        if (Objects.nonNull(task)){
            String name = task.getName();
            if (StringUtils.isEmpty(name)){
                return "unDefined";
            }
             return name;
        }
        return "unDefined";
    }

我们尝试使用Optional进行代码简化优化之后，是不是看上去立马简洁很多了？

public String getTaskName(Task task){
                return Optional.ofNullable(task).map(p->p.getName()).orElse("unDefined");
    }

总结

本文主要和大家聊了聊日常工作中比较常见的奇葩代码，当然吐槽并不是目的，研发同学能够识别到奇葩代码并进行优化，同时自己在实际开发工程中能够尽量避免写这些代码才是真正的目的。不知道大家在工作中有没有遇到过类似的奇葩代码或者自己曾经写过哪些现在回过头来看比较奇葩的代码，如果有的话欢迎大家在评论区一起讨论交流哈。

文章详情

聊聊那些年遇到过的奇葩代码