Ticket #45 (closed QA: fixed) — at Version 5

Opened 14 years ago

Last modified 14 years ago

一个拼接sql的小问题

Reported by: chenyinle Owned by:
Priority: major Milestone:
Component: 商家后台 Version:
Keywords: 常见故障 Cc:
Due Date: 06/12/2011

Description (last modified by chenyinle) (diff)

首先,这只是一个很小很小的细节问题,但我们可以于细微处见精神,发散一下,如...

是这样的,今天处理一个数据接口的报错的bug,找啊找,终于找到了,原来是sql的字符串拼接错误

错误来源是拼的sql的in条件格式出错,具体表现为例如:select * from table where in(111,234,),所以导致sql执行错误了

原代码如下:
http://rdwiki.pc.com.cn/download/attachments/1015925/2222222222.jpg?version=2&modificationDate=1323059395000

其实,一咋眼,感觉没问题,but,实际上套红的201行的判断逻辑是不严谨的

这个方法涉及到四点问题,一个效率低的问题,和三个不严谨的地方

1、一个性能的问题。201行,仅仅是为了判断是否为list里面最后一个遍历的元素,其实不用indexOf(indexOf是需要遍历list的),这里的时间复杂度是O(n*n).

so,从性能上来说,这里可以改成for(;;)这种遍历方式,直接判断下标是否是最后一个元素就行了,这样时间复杂度是O(n).

2、严谨性的问题。由于for循环里面199行还有一个判断(news.getUser.getCompany!=null,应该算是有效元素的一个判断吧),这样的话,遍历的时候执行到201行怎样也无法保证是否遍历到最后一个元素的(因为外面还有一层判断,也许会提前结束遍历)。

3、第二个严谨性的问题。list.indexOf其实是不能保证当前遍历的元素和indexOf的返回下标一致,因为indexOf默认情况下只会比较对象引用是否相等,假如list里面包含两个一样的元素(同一个对象),这样会导致代码判断逻辑出错。

4、第三个严谨性的问题。当有一种极端的情况,list传进来为null,或每一次news.getUser.getCompany!=null判断的时候,都返回false,那么方法返回值就是'()'了,格式不正确,应该返回'(11,22,33)'格式。

事实上,这次的出错的正是第三点的不严谨导致的,因为一些原因,方法里面传入的list的元素真的有重复了,导致indexOf的时候返回的下表不准了,就判断错误,导致方法返回了'(1,2,)'这种格式了。

so,就这个方法而言,做了如下改进:

http://rdwiki.pc.com.cn/download/attachments/1015925/111.jpg?version=1&modificationDate=1323059395000

同时,传入的list本身保存了重复的对象元素,也修正了这个bug。

改进了之后,
1、保证了效率问题,时间复杂度是O(n),且只管遍历就得了,不用每次都判断是否为最后一个元素
2、由于in里面是ID参数,so,假如in括号的参数为空时,返回了'(0)',满足了格式要求,且不会导致数据不一致(这里只是一种方式,不是说这样做就很好很官方)
3、保证了返回的字符串逻辑严谨性

不管如何,这样bug也修复了,代码也严谨了...

Change History

comment:1 Changed 14 years ago by chenyinle

  • Status changed from new to closed
  • Type changed from defect to QA
  • Resolution set to fixed

comment:2 Changed 14 years ago by chenyinle

  • Keywords 常见故障 added

comment:3 Changed 14 years ago by huangzhong

很赞同这样的修改,循环里尽量少做事情,改完之后的代码很清晰

comment:4 Changed 14 years ago by chenyinle

  • Description modified (diff)

comment:5 Changed 14 years ago by chenyinle

  • Description modified (diff)
Note: See TracTickets for help on using tickets.