本来是要修复前一个代码bug,修复的过程中发现原本的代码又丑又长,复用性差(**但是能用**),出于强迫症忍不住的去优化,测试还不充分,火急火燎的发到生产了,结果掉井了!导致多个订单线下物流发货发多了.... 万一有个别用户不管订单数量是不是自己下单的,直接签收了,再往回要就难了,那时还要加上来回运费。 当时提桶跑路的心都有/(ㄒoㄒ)/~~,但这个东西逃不掉,而且问题越拖越严重。这个时候就不能再去顾虑别人对自己的看法了,事情已经这样了,可奈何 ?不如主动承认错误,尽早解决降低损失。 最后硬着头皮向领导说明了情况,然后联系协助各方人员尽量挽救,将损失降到最低。 整个过程真是自惭形秽,无地自容,恨不能找个地缝钻进去 ... **事后总结:** 当时因为需要构建好多场景,嫌麻烦,加上蜜汁自信,测试不充分。 主观上是因为自己对于代码失去了敬畏之心,随意修改,不考虑或者不想考虑如果错了会造成什么样的后果。 ==对于代码最好的优化就是**能跑通就不要优化**。如要修改,选择对于已有代码影响最小的方案。== 下面模拟这个问题 ## 准备 目的:将集合中重复的元素合并累加起来 比如:将下面列表 bigHouseList 中相同房屋主人 ownerName 的房屋价值 worth 累加起来 ```java BigHouse house1 = new BigHouse("张三", "别墅", new BigDecimal("2400"), "想不到路-10号"); BigHouse house2 = new BigHouse("赵柳", "公寓", new BigDecimal("120"), "现实路30号"); BigHouse house3 = new BigHouse("李四", "别墅", new BigDecimal("780"), "你别想路-101号"); BigHouse house4 = new BigHouse("王五", "小洋楼", new BigDecimal("8000"), "梦不到路-1000号"); BigHouse house5 = new BigHouse("赵柳", "别墅", new BigDecimal("3500"), "想不到路-28号"); BigHouse house6 = new BigHouse("王五", "别墅", new BigDecimal("4200"), "想不到路-130号"); List bigHouseList = new ArrayList<>(); bigHouseList.add(house1); bigHouseList.add(house2); bigHouseList.add(house3); bigHouseList.add(house4); bigHouseList.add(house5); bigHouseList.add(house6); ``` **大房子类 BigHouse** ```java @Setter @Getter public class BigHouse { /** 房屋主人名字 */ private String ownerName; /** 房屋类型 */ private String type; /** 房屋价值(单位万) */ private BigDecimal worth; /** 详细地址 */ private String address; public BigHouse() { } public BigHouse(String ownerName, String type, BigDecimal worth, String address) { this.ownerName = ownerName; this.type = type; this.worth = worth; this.address = address; } } ``` ## 优化前的代码 > 优化理由:这部分代码逻辑和业务无关,可以抽离出来形成一个公共的方法函数 ```java //region 优化前的代码 List uniqueOwnerHouseList = new ArrayList<>(); boolean isSameOwner = true; for (BigHouse house : bigHouseList) { isSameOwner = Boolean.FALSE; if (uniqueOwnerHouseList.size() > 0) { for (BigHouse h : uniqueOwnerHouseList) { if (h.getOwnerName().equals(house.getOwnerName())) { BigDecimal sumWorth = h.getWorth().add(house.getWorth()).setScale(0, BigDecimal.ROUND_HALF_UP); h.setWorth(sumWorth); isSameOwner = Boolean.TRUE; } } } if (!isSameOwner) { uniqueOwnerHouseList.add(house); } } //endregion System.out.println("优化前结果输出:"); //优化前结果输出 for (BigHouse bigHouse : uniqueOwnerHouseList) { System.out.println(bigHouse.getOwnerName() + "坐拥 " + bigHouse.getWorth() + " 万房产!"); } ``` 输出结果: ``` 优化前结果输出: 张三坐拥 2400 万房产! 赵柳坐拥 3620 万房产! 李四坐拥 780 万房产! 王五坐拥 12200 万房产! ``` ## 优化后的代码(bug) ```java /** * 合并集合中重复的元素 * * @param items * @param predicate 判断是否重复 * @param consumer 重复后处理 * @param * @return */ public static List mergeDuplicate(Collection items, BiPredicate predicate, BiConsumer consumer) { List mergeItems = new ArrayList<>(); for (T item : items) { boolean existSameItem = false; for (T mergeItem : mergeItems) { existSameItem = predicate.test(mergeItem, item); if (existSameItem) { consumer.accept(mergeItem, item); } } if (!existSameItem) { mergeItems.add(item); } } return mergeItems; } //优化后的代码 List mergedBigHouseList = CollectionTools.mergeDuplicate(bigHouseList, (p, n) -> p.getOwnerName().equals(n.getOwnerName()), (p, n) -> p.setWorth(p.getWorth().add(n.getWorth()).setScale(0, BigDecimal.ROUND_HALF_UP)) ); System.out.println("优化后结果输出:"); for (BigHouse bigHouse : mergedBigHouseList) { System.out.println(bigHouse.getOwnerName() + "坐拥 " + bigHouse.getWorth() + " 万房产!"); } ``` 输出结果: ``` 优化前结果输出: 张三坐拥 2400 万房产! 赵柳坐拥 3620 万房产! 李四坐拥 780 万房产! 王五坐拥 12200 万房产! 赵柳坐拥 3500 万房产! 王五坐拥 4200 万房产! ``` ## 逻辑分析 原因出在这块代码上 ```java for (T mergeItem : mergeItems) { existSameItem = predicate.test(mergeItem, item); if (existSameItem) { consumer.accept(mergeItem, item); } } ``` 在 if 判断true后,**应该break跳出循环**的,否则 existSameItem 的值会被 mergeItems 剩余元素的比较结果覆盖掉,从而影响循环外的 if 判断 ```java if (!existSameItem) { mergeItems.add(item); } ``` 正确的写法: ```java for (T mergeItem : mergeItems) { existSameItem = predicate.test(mergeItem, item); if (existSameItem) { consumer.accept(mergeItem, item); break; } } ``` 或者(不建议) ```java for (T mergeItem : mergeItems) { if (predicate.test(mergeItem, item);) { consumer.accept(mergeItem, item); existSameItem = true; } } ```