标签：重构 commerceItem items 代码 item 质量 && currentId
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);
}
}
}
}
标签：重构,commerceItem,items,代码,item,质量,&&,currentId 来源： https://www.cnblogs.com/gocode/p/refactor02.html
本站声明： 1. iCode9 技术分享网（下文简称本站）提供的所有内容，仅供技术学习、探讨和分享； 2. 关于本站的所有留言、评论、转载及引用，纯属内容发起人的个人观点，与本站观点和立场无关； 3. 关于本站的所有言论和文字，纯属内容发起人的个人观点，与本站观点和立场无关； 4. 本站文章均是网友提供，不完全保证技术分享内容的完整性、准确性、时效性、风险性和版权归属；如您发现该文章侵犯了您的权益，可联系我们第一时间进行删除； 5. 本站为非盈利性的个人网站，所有内容不会用来进行牟利，也不会利用任何形式的广告来间接获益，纯粹是为了广大技术爱好者提供技术内容和技术思想的分享性交流网站。