2007-08-20

Java Blooper #2: Must be a Better Way...

It's Monday, which means it's time for another blooper... What's wrong with this code?

boolean hasThing( List things, Thing thing ) {
  for ( int i=0; i < things.size(); i++ ) {
    if ( thing.equals( things.get( i ) ) ) {
      return true;
    }
  }
  return false;
}
Update: Minor edit to add missing parenthesis from if statement that got "lost in translation" en-route to the blog :)

AddThis Social Bookmark Button

13 comments:

Carsten said...

Hm, it doesn't use List#contains(Object)?

IDD said...

Seems the contains is the simplest solution. Btw it's also missing a closing ')' at the end of the if condition, but I guess I'm just picky ;-)

Mike Kaufman said...

Yes, use "contains", but I think the actual "blooper" is inefficient random access into the List - each get(i) potentially involves reading through the List from scratch to reach the ith element.

From List's javadoc: "...may execute in time proportional to the index value for some implementations (the LinkedList class, for example). Thus, iterating over the elements in a list is typically preferable to indexing through it if the caller does not know the implementation".

So "better way" would be to use the List's iterator (for-each or iterator/hasNext/next).

Using "contains" is even better ("tell, don't ask"), depending on how you want to handle a null "thing" (the given code doesn't check for it and would throw NPE; "contains" would match a null element in the List).

Alex said...

Using contains() would also be better if the collection is being used in a multi-threaded environment and the collection is using a synchronized wrapper and/or a multi-thread safe collection. In that case, this code could be broken or give you unexpected results (seeing same value twice, ConcurrentModificationException, ArrayIndexOutOfBoundsException, etc).

Jeff said...

if either things or thing is null...doh!

Olaf said...

I usually don't like to call size() in a loop. It slows things down (Image you're using a Vector here).

Using size() will have concurrency issues if the list is shared among many threads.

Guy Mac said...

Well, it depends on your definition of "in the list". By using the equals() method you're asking if the Thing is equal to any Thing in the list, but not necessarily if it is actually the same instance as any in the list.

Anonymous said...

wow gold
cheap wow gold
wow guide
nba live
火箭队
google排名
buy wow gold
world of warcraft gold
runescape
google左侧排名
wow gold
runescape money
lotro
lotro gold
guild wars
guild wars gold
google排名服务
dofus
dofus kamas
world of warcraft gold
maple story
maple story mesos
maple story
maple story mesos

google优化 said...

无锡乐洋化机公司主要采购反应设备销售反应设备反应设备商机反应设备产品反应设备公司反应设备供应商反应设备市场反应设备价格行情反应设备展会信息反应设备行业资讯反应设备反应设备本公司主要生产反应设备销售反应设备制造反应设备和各种产品我们是反应设备供应商有很大的反应设备市场反应设备详细情况可以访问反应设备专业网本公司主要生产反应设备
销售反应设备冷凝器冷凝器冷凝器冷凝器冷凝器反应锅反应锅反应锅反应锅反应锅反应釜反应釜反应釜反应釜反应釜反应釜反应釜反应釜反应釜搅拌设备搅拌设备搅拌设备不锈钢反应釜冷凝器冷凝器冷凝器冷凝器冷凝器展会信息冷凝器行业资讯反应锅反应锅反应锅反应锅反应锅反应釜反应釜反应釜反应釜反应锅反应釜换热器换热器换热器换热器

Anonymous said...

有什么 有什么网址 有什么新闻 有什么博客 有什么论文 有什么图片 有什么音乐 有什么搜商 有什么帖客 天气预报

Anonymous said...

runescape money runescape gold runescape money runescape gold wow power leveling wow powerleveling Warcraft Power Leveling Warcraft PowerLeveling buy runescape gold buy runescape money runescape items runescape gold runescape money runescape accounts runescape gp dofus kamas buy dofus kamas Guild Wars Gold buy Guild Wars Gold lotro gold buy lotro gold lotro gold buy lotro gold lotro gold buy lotro gold runescape money runescape power leveling runescape money runescape gold dofus kamas cheap runescape money cheap runescape gold Hellgate Palladium Hellgate London Palladium Hellgate money Tabula Rasa gold tabula rasa money lotro gold buy lotro gold Tabula Rasa Credit Tabula Rasa Credits Hellgate gold Hellgate London gold dofus kamas buy dofus kamas wow power leveling wow powerleveling Warcraft PowerLeveling Warcraft Power Leveling World of Warcraft PowerLeveling World of Warcraft Power Leveling runescape power leveling runescape powerleveling eve isk eve online isk eve isk eve online isk 成都网站优化 成都网站推广 北京网站推广 北京网站优化 血管瘤 肝血管瘤 音乐剧 北京富码电视 富码电视 富码电视台 7天酒店 7天连锁酒店 7天连锁 自清洗过滤器 过滤器 压力开关 压力传感器 流量开关 流量计 液位计 液位开关 温湿度记录仪 风速仪 可燃气体检测仪

Anonymous said...

wow gold
wow gold
wow gold
wow gold
wow gold
wow gold
wow gold
World of Warcraft Gold
World of Warcraft Gold
wow power leveling
wow power leveling
wow power leveling
wow power leveling
wow power leveling
wow powerleveling
wow powerleveling
wow power leveling

wow power leveling
powerleveling
powerleveling
powerleveling
powerleveling
power leveling
power leveling
power leveling
power leveling
rolex replica
rolex
replica rolex
wow power leveling
wow power leveling
wow powerleveling
wow powerleveling
wow powerleveling
wow powerleveling
wow powerleveling
wow powerleveling
wow powerleveling
powerleveling
powerleveling
powerleveling
powerleveling
power leveling
power leveling
power leveling
power leveling

中高年 転職
アルバイト 求人情報
ブライダル
転職
競馬
FX
ダイエット
お見合い
競馬 予想
新築マンション
新築マンション
コンタクトレンズ
合宿免許
人材派遣
東京都 墓地
派遣会社
人材派遣
パチンコ 攻略
おなら

货架
翻译公司
翻译公司
机票
性病
性病
尖锐湿疣
尖锐湿疣
蜗轮减速机
减速机
齿轮减速机
丝杆升降机
租房
租房
北京租房
北京租房
搬家公司
北京搬家
北京搬家公司
上海机票
上海机票
上海打折机票
上海打折机票
上海特价机票

上海特价机票
搬家公司
搬家公司
北京搬家公司
北京搬家公司
窃听器
窃听器
手机窃听器
手机窃听器
代孕
试管婴儿
捐卵
代孕
试管婴儿
试管婴儿
捐卵
捐卵
代孕
试管婴儿

maplestory mesos said...

lotro
lotro gold
lotro money
lord of the ring
lotro
lotro gold
lotro money
lord of the ring
maplestory
maplestory mesos
buy maplestory mesos
cheap maplestory mesos
wow gold
buy wow gold
cheap wow gold
wow
wow power leveling
wow powerleveling
world of warcraft
world of warcraft gold
world of warcraft power leveling
world of warcraft powerleveling