码迷,mamicode.com
首页 > 其他好文 > 详细

工作中的重构:提高代码质量(二)

时间:2020-02-02 21:48:37      阅读:83      评论:0      收藏:0      [点我收藏+]

标签:ceo   eof   erro   结果   stack   commit   问题   collect   car   

 

1.代码逻辑不清晰

  • origin
            CommerceItem mergeItem = null;
            List items = getNgpCartModifierFormHandler().getOrder().getCommerceItemsByCatalogRefId(baseCommItem.getCatalogRefId());
            if (items != null && items.size() > 1) {
                Iterator key = items.iterator();
                while (key.hasNext()) {
                    NGPCommerceItemImpl item = (NGPCommerceItemImpl) key.next();
                    if (!item.getRepositoryItem().getRepositoryId().equals(baseCommItem.getRepositoryItem().getRepositoryId()) && item.getWarrantyItem() == null
                        && !getQuoteTools().isQuoteItem(baseCommItem) && !getQuoteTools().isQuoteItem(item)) {
                        mergeItem = item;
                    }
                }
            }

 

上述代码存在的问题:
①对常用API不够熟悉,“items != null && items.size() > 1”只是对集合item进行至少有一个元素的判断,可用“CollectionUtils.size(items)>1”代替,使代码更精炼可读。
②“item.iterator()“可用forEach代替,forEach反编译后的字节码就用使用迭代器,使用foreach在源码上看上去更精炼。
③变量名字不规范,”item.iterator()“的返回值是一个迭代器类型对象,应用”itor"或"iterator”,而“keys”代表一个键的集合,只有map类型有这个概念,这里用“keys”命名容易让人产生误解。
④if判断条件太长,让人不知道具体要判断什么东西
⑤代码表达错误的意图
while循环中mergeItem 最终的值是最后一次满足if条件的循环中所赋的值,以前的遍历赋值的结果会被最后的遍历给覆盖掉,之前的遍历中的if条件判断是没有意义的,浪费系统资源。如果写代码的程序员真实的意图就是这样,他应该从后向前遍历,满足if条件则退出循环。而后面了解相关业务后才知道,理论上这个item集合中最多只有一个元素满足if条件,而原代码的写法明显没有表现出这种意图,容易让人产生误解,也浪费了系统资源。

  • corrected
        CommerceItem mergeItem = null;
            List items = getNgpCartModifierFormHandler().getOrder().getCommerceItemsByCatalogRefId(baseCommItem.getCatalogRefId());
            if (CollectionUtils.size(items)>1) {
                for (Object item : items) {
                    if (item instanceof NGPCommerceItemImpl) {
                        NGPCommerceItemImpl itemImpl = (NGPCommerceItemImpl) item;
                        boolean isMerged = !itemImpl.getRepositoryItem().getRepositoryId().equals(baseCommItem.getRepositoryItem().getRepositoryId())
                            && itemImpl.getWarrantyItem() == null
                            && !getQuoteTools().isQuoteItem(baseCommItem) 
                            && !getQuoteTools().isQuoteItem(itemImpl);
                        if (isMerged) {
                            mergeItem = itemImpl;
                            break;
                        }
                    }

                }
            }

 

2.变量名表达的含义南辕北辙

谁能看出来下面的代码要干啥,代码片段“Long.parseLong(currentId) != commerceItem.getQuantity()”在将商品唯一标识id与商品数量比较是否相等,这两个完全不在一个维度的概念可以进度比较吗,它们的比较有什么意义。
后来通过与同事交流,才知道代码片段“pRequest.getParameter(commerceItem.getId())“背后的含义,前端将商品id作为参数名、商品最新数量为参数值的方式传递到后端,那么此时其返回值叫作”currentId“就非常不全时宜了,应该叫做"modifiedQuantity"或"modifiedQuantityForTextFormat”。

  • origin:
    for (CommerceItem commerceItem : commerceItems) {
            String  currentId= pRequest.getParameter(commerceItem.getId());
            if (StringUtils.isNotBlank(currentId)) {
                if (commerceItem instanceof GCNGPLessonsCommerceItemImpl) {
                    if (Long.parseLong(currentId) != commerceItem.getQuantity()) {
                      vlogError("preSetOrderByCommerceId() Lessons quantity can not be changed to:{0}", currentId);
                        //...省略相关代码
                    }
                }
            }
        }

 

  • corrected
        for (CommerceItem commerceItem : commerceItems) {
            String modifiedQuantityForTextFormat = pRequest.getParameter(commerceItem.getId());
            if (StringUtils.isNotBlank(modifiedQuantityForTextFormat)) {
                if (commerceItem instanceof GCNGPLessonsCommerceItemImpl) {
                    if (Long.parseLong(modifiedQuantityForTextFormat) != commerceItem.getQuantity()) {
                        vlogError("preSetOrderByCommerceId() Lessons quantity can not be changed to:{0}", currentId);
                    }
                }
            }
        }

 




工作中的重构:提高代码质量(二)

标签:ceo   eof   erro   结果   stack   commit   问题   collect   car   

原文地址:https://www.cnblogs.com/gocode/p/refactor02.html

(0)
(0)
   
举报
评论 一句话评论(0
登录后才能评论!
© 2014 mamicode.com 版权所有  联系我们:gaon5@hotmail.com
迷上了代码!