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

12 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

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