代码的细节

February 24, 2019
重构 重构的必要性

字数:6137

引文

曾听说把一件事情做到极致,或许就是取胜之道其一。本文不是想要论述这个道理,只有感于组员在工作中细节部分不够到位,这种不到位并不是方案 的问题、也不是技术深度的问题,仅仅就是有没有多想一点,类似的总结,这里也有。虽然项目工作量大,人力吃紧,但是 做任何东西先要自己这关吧?

1 起因

后台管理程序要开发一个接口,功能就是将绘本的信息入库。包括几大块:

本文要聊的就是组员在实现后面4个一对多时的一些细节做法,以及存在的问题,这些东西我也当然找他们聊了,以求后续工作中不出现这种比较一般的问题。

2 问题

2.1 参数的问题

下面是书籍获得的奖项参数,用对象封装:

@Valid
public class FrmBookAward {

    @Column(name="id")
    @JsonView(SimpleView.class)
    long id;
    
    @Column(name="id_book")
    @JsonView(SimpleView.class)
    long idBook;

    @Column(name="id_award")
    @JsonView(SimpleView.class)
    long idAward;

2.2 更新操作的问题

下列是执行具体更新的代码:

 /**
 * 图书表对应奖项信息
 * @param frmBookAwards
 * @param idBook
 */
@Override
//标示4,由于此方法内更新了数据,是可以直接更新缓存的,而这里选择剔除缓存,是为了下次查的时候再次缓存,有点回避。
更关键的是,这里缓存的是对应关系,而奖项的基本信息,比如名称,还需要再次查询,何不直接缓存奖项的基本信息,这个基本信息才是被大量读的内容
@CacheEvict(value = "ggh", key = "'book_award_'.concat(#idBook)")
public void saveAllAward(List<FrmBookAward> frmBookAwards, long idBook) {
    List<BookAward> bookAwardList = new ArrayList<>();
    List<Long> ids = new ArrayList<>();
    for (FrmBookAward frmBookAward : frmBookAwards) {
        BookAward bookAward = utils.createEntity(BookAward.class);
        bookAward.setIdBook(idBook);
        if(frmBookAward.getId() == 0){
            BeanUtils.copyProperties(frmBookAward, bookAward, ArrayUtils.add(CommConst.DO_NOT_REPLACE, "idBook"));
        }else{
            BeanUtils.copyProperties(frmBookAward, bookAward, ArrayUtils.add(CommConst.DO_NOT_REPLACE_ID, "idBook"));
        }
        bookAwardList.add(bookAward);
        //添加ids
        ids.add(bookAward.getIdAward());
        //标示2,如果里面有错误id,即后续的检验通不过,则此为不必要的构造对象列表 
    }
    //查询数据库中是否存在id_category
    int listSize = awardRepository.countByIdIn(ids);
    //如果二者的数值不相等,可以认为有数据库不存在的id存在
    if(listSize != bookAwardList.size() ){
        throw  new  MyException(400,"奖项id有误");
    }
    //查询数据库中存在的信息,对比传入值,如果id对应不上则删除
    //标示5,findBookAwardByIdBook原本想读缓存里的内容,而在同一个类里,是无效的
    List<BookAward> bookAwards = findBookAwardByIdBook(idBook);
    List<BookAward> notExists = new ArrayList<>();
    for (BookAward bookAward : bookAwards) {
        //是否存在
        boolean f =false;
        //标示3,这里才是最无聊的,bookAwardList里构造的对象的ID全是重新生成的,是不可能相等的,所以
        结果就是数据库中的每条数据都会被逻辑删除
        for (BookAward paramBookAward : bookAwardList){
            if(bookAward.getId() == paramBookAward.getId()){
                f = true;
                //标示1,这里是可以break跳出的
            }
        }
        if(!f){
            bookAward.setDelete(true);
            notExists.add(bookAward);
        }
    }
    bookAwardList.addAll(notExists);
    bookAwardRepository.saveAll(bookAwardList);
}

为了解决这个问题,如果仅id数据都还好办,而如果像书籍作者,对应信息有type(类型,比如著、图、文;作者姓名;国别),如何比对对象呢? 图方便的话,修改一下对应的equals和hashCode方法

2.3 更新没考虑业务的特点

书籍与价格的一对多,小伙也是如出一辙的处理,其实不能这样,价格有很多种——’原借阅价’,‘现借阅价’,‘会员价’,‘活动价1’,‘活动价2’,‘活动价3’, 而在这个接口对应的前端页面只涉及到其中的部分,比如’原借阅价’,‘现借阅价’。

3 第1次重构

基于以上的问题,所以我实现的版本的是:

@Override
@Caching(put = {
        @CachePut(value = "ggh", key = "'book_awards_'.concat(#bookAwards.get(0).idBook)")
})
public List<Award> saveBookAwards(List<BookAward> bookAwards) {
    if (bookAwards == null || bookAwards.isEmpty()) return null;
    //更新关联
    bookAwardRepository.saveAll(bookAwards); //这里可能含有isdelete的数据
    Iterable idsAward = new ArrayList<>();

    for (BookAward bookAward : bookAwards) {
        if (!bookAward.isDelete()) {
            ((ArrayList) idsAward).add(bookAward.getIdAward());
        }
    }
    //查awards基本信息
    List<Award> awards = (List<Award>)awardRepository.findAllById(idsAward);
    return awards;
}

而业务判断放在service方法中:

//3 保存奖项
long[] frmBookAwardIds = frmBook.getBookAwards();
boolean isEmptyFrmBookAwardIds = (frmBookAwardIds == null) || (frmBookAwardIds.length == 0);

//检查所有奖项编号是否合法
if (!isEmptyFrmBookAwardIds) {
    int validAmountExists = awardRepository.countByIdIn(frmBookAwardIds);

    if (validAmountExists != frmBookAwardIds.length) {
        throw new MyException(400, "存在非法奖项id");
    }
}
if (frmBookAwardIds == null) { //初始化后,可直接用
    frmBookAwardIds = new long[0];
}
List<BookAward> existsBookAwards = bookAwardRepository.findByIdBook(existsBook.getId());
boolean isEmptyExistsBookAwards = (existsBookAwards == null) || (existsBookAwards.isEmpty());

if (!isEmptyExistsBookAwards) {
    for (BookAward existsBookAward : existsBookAwards) {
        if (!ArrayUtils.contains(frmBookAwardIds, existsBookAward.getIdAward())) {
            existsBookAward.setDelete(true);
        } else {
            //已比对的,排除,剩下的即数据库中不存在,要新增
            frmBookAwardIds = ArrayUtils.removeElement(frmBookAwardIds, existsBookAward.getIdAward());
        }
    }
}
if (frmBookAwardIds.length > 0 && existsBookAwards == null) {
    existsBookAwards = new ArrayList<>();
}

//新增
for (long frmBookAwardId : frmBookAwardIds) {
    BookAward newBookAward = utils.createEntity(BookAward.class);
    newBookAward.setIdBook(existsBook.getId());
    newBookAward.setIdAward(frmBookAwardId);
    newBookAward.setDelete(false);
    newBookAward.setLastTime(new Timestamp(System.currentTimeMillis()));
    existsBookAwards.add(newBookAward);
}
//更新到数据库中
if (existsBookAwards != null && !existsBookAwards.isEmpty()) {
    cacheMgrService.saveBookAwards(existsBookAwards);
    existsBook.setBookAwards(cacheMgrService.findAwardsOfBook(existsBook.getId()));
}

4 第2次重构

虽然第1次重构使逻辑清晰化了,但是后来在用的过程中还是发现有一点点不能满足使用:

这样第2次重构又势在必行了,还是以上述的奖项为例,核心重构代码比如变为:

@Override
@Caching(put = {
        //直接更新书籍缓存,而非对应关系的原生数据
        @CachePut(value = "ggh", key = "'book_idbook_'.concat(#newBook.id)"),
        @CachePut(value = "ggh", key = "'book_isbn_'.concat(#newBook.publishIsbn)")
})
public Book saveBookAwards(Book newBook) {
    //把要更新的对应关系数据(数据库和表单双向对比之后的确认结果)取出来
    List<BookAward> bookAwards = newBook.getBookAwardsRelative();

    if (bookAwards == null || bookAwards.isEmpty()) return null;
    //更新关联
    bookAwardRepository.saveAll(bookAwards); //这里可能含有isdelete的数据
    Iterable idsAward = new ArrayList<>();

    for (BookAward bookAward : bookAwards) {
        if (!bookAward.isDelete()) {
            ((ArrayList) idsAward).add(bookAward.getIdAward());
        }
    }
    //查awards基本信息
    List<Award> awards = (List<Award>) awardRepository.findAllById(idsAward);
    newBook.setBookAwards(awards);
    return newBook;
}

5 结语

由上可见,虽然功能简单,如没想仔细,也可能做岔了,程序不健壮,事情还得反复。


loading