一个拼接sql的小问题

[chenyinle]

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

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

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

原代码如下：

其实，一咋眼，感觉没问题，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，就这个方法而言，做了如下改进：

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

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

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

[huangzhong]

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

[liaojunqiang]

补充一下：where in ('xx','xx'..) ‘xx’只能写1000次，oracle下

[huangzhong]

select * from
( select a.* from v_ent_product a, ent_company c where a.user_id=c.id and a.editor_small_recommend=1

and a.price_status<>1 and a.status<=1 and a.sell_status=1 and c.status>=0 and c.vip_rank>0
and a.type_id in(SELECT id FROM V_PDL_PRODUCT_TYPE START WITH id =:1 CONNECT BY parent_id= PRIOR id) order by a.last_update_date desc) where rownum<=200;

在代码中看到类似的拼接sql语句，其中a.type_id in(SELECT id FROM V_PDL_PRODUCT_TYPE START WITH id =:1 CONNECT BY parent_id= PRIOR id)的in的范围是由递归方式获得的，这样的sql性能完全控制不了