Code Review 总结一

最新推荐文章于 2024-06-19 22:05:15 发布

Prayszzz

最新推荐文章于 2024-06-19 22:05:15 发布

阅读量1.9k

点赞数 1

分类专栏：工作总结文章标签：团队

本文链接：https://blog.csdn.net/u013096145/article/details/52107411

版权

工作总结专栏收录该内容

0 篇文章 0 订阅

订阅专栏

就在08-02昨天，我们团队进行了一次代码review大会,此博客记录自己写代码时候被指出的问题，以免再犯同样的错误。

1.直接返回结果，难调试

return ApiResult.success(userService.get(userId));

最开始的写法如上所示，其中userService返回的是user对象，ApiResult.success会将对象通过Json返回给前端。
虽然在调试的过程中可以通过右键 Add to Watches查看该user对象(使用的是Idea)，但还是不直观，不好调。最终最直观的方法如下：

User user = userService.get(userId);
return ApiResult.success(user);

2.代码结构太长

在最近的一次迭代中，有一个service方法由于要拿很多数据就写了120行左右，代码都放在一起很难维护，其他开发人员看的欲望都没了。
通过今天简单的重构一下，将获取相应数据模块的代码抽取出来以后，每个方法都在40行以内，降低了阅读成本，代码也更优雅了。

3.查询数据库次数过多

 for(Integer id : ids){
     userService.get(id);
 }

由于项目是通过Jar包依赖，再多加个接口，需要重新打包发布依赖。
有些时候为了偷懒，会像上面这样写，这样就不需要在写一个接口了，这样导致的结果就是一个简单的接口查询太多次数据库，当项目越来越大以后，访问量越来越大，数据库可能就撑不住了。

4.缺少应有的空行

        HiVO hiVO = new VO();
        User user = userService.get(userId);
        if(user == null)
            return null;
        hiVO.setGender(user.getGender());
        hiVO.setUserId(userId);
        hiVO.setUserName(user.getUserNickName());
        hiVO.setPosition(user.getPosition());
        int times = hiService.getTimes(userId);
        hiVO.setTimes(times);
        String url = fileResourceService.getUrl(user.getHeaderPicId());
        hiVO.setHeadPicUrl(url);
        List<UserPhotoDTO> userPhoneDTOs = userPhotoService.getByUserId(userId);
        if(userPhoneDTOs != null){
            for(UserPhotoDTO userPhotoDTO : userPhoneDTOs){
                if(userPhotoDTO.isCover()){
                    hiVO.setCoverPicUrl(userPhotoDTO.getPicUrl());
                }
            }
            hiVO.setPhotos(userPhoneDTOs);
        }

上面的代码一眼看去是知道在填充数据，由于没有注释，要看哪里填充了什么数据就有些不太明显了，这个时候适当的添加个空行能起到注释的作用。
如下图所示：首先填充用户的基本信息，再填充service信息，头像信息等，一目了然。

        HiVO hiVO = new VO();

        User user = userService.get(userId);
        if(user == null)
            return null;
        hiVO.setGender(user.getGender());
        hiVO.setUserId(userId);
        hiVO.setUserName(user.getUserNickName());
        hiVO.setPosition(user.getPosition());

        int times = hiService.getTimes(userId);
        hiVO.setTimes(times);

        String url = fileResourceService.getUrl(user.getHeaderPicId());
        hiVO.setHeadPicUrl(url);

        List<UserPhotoDTO> userPhoneDTOs = userPhotoService.getByUserId(userId);
        if(userPhoneDTOs != null){
            for(UserPhotoDTO userPhotoDTO : userPhoneDTOs){
                if(userPhotoDTO.isCover()){
                    hiVO.setCoverPicUrl(userPhotoDTO.getPicUrl());
                }
            }
            hiVO.setPhotos(userPhoneDTOs);
        }

5.省略{}

for(Integer id : userIds){
    User user = userService.get(id);
    if(user.getTime() > hiUser.getTime())
        return user.getTime();
    else if(user.getTime() < hiUser.getTime())
        return hiUser.getTime();
}

一开始觉得if省略{}更加简洁，但这是个不好的习惯，别人看着会很累，改也容易犯错。关键的是，在很长的代码中嵌入这样的代码，那就更加难以排查错误，为了降低阅读成本和出错率的话还是把{}加上吧。

for(Integer id : userIds){
    User user = userService.get(id);
    if(user.getTime() > hiUser.getTime()){
        user.getTime();
    }else if(user.getTime() < hiUser.getTime()){
        return hiUser.getTime();
    }
}

6.命名不直观

Collections.sort(userDTOs, new Comparator<User>() {
            @Override
            public int compare(User o1, User o2) {
                if(o1.getTimes() > o2.getTimes()){
                    return -1;
                }else if(o1.getTimes() == o2.getTimes()){
                    return 0;
                }
                return 1;
            }
});