• Post Reply Bookmark Topic Watch Topic
  • New Topic

iterator elegance  RSS feed

 
Dorcas Rebanha
Greenhorn
Posts: 18
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
This code seems to work, but I think there is a redundant line in it. Please educate me.

List thingList = this.retrieveListOfThings();
if (thingList.size() > 0 ) {
Iterator thingIter = thingList.iterator();
while (thingIter.hasNext()) {
... do interesting stuff here
}
}

I believe the "if (thingList.size() > 0 )" condition is unnecessary because the "while" loop will simply do nothing if the list size is 0. Am I correct?

Thanks.
 
fred rosenberger
lowercase baba
Bartender
Posts: 12565
49
Chrome Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
What happens when you try taking it out, and make a list with no elements?
 
Jesper de Jong
Java Cowboy
Sheriff
Posts: 16060
88
Android IntelliJ IDE Java Scala Spring
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If you are using Java 5 or newer, you can use the new "foreach" loop instead of explicitly dealing with the iterator, which makes your code much shorter and cleaner:

Also, if you're using Java 5 or newer, it's a good idea to use generics, so that you don't need to cast from Object to whatever type of things your list contains. I don't know what kinds of objects your list contains, but suppose it contains strings, then it would be something like this:
 
chandrakant karale
Ranch Hand
Posts: 41
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I believe the "if (thingList.size() > 0 )" condition is unnecessary because the "while" loop will simply do nothing if the list size is 0. Am I correct?


Yes thingList.size()> 0 is redundent.
But instead you should check for null value of thingList before you proceed with

 
Peter Chase
Ranch Hand
Posts: 1970
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Originally posted by chandrakant karale:
But instead you should check for null value of thingList before you proceed


Only if null is an allowed value of that thing at that point. (We can't know, just from the small code fragment).

Too much (beginner) code is burdened with loads of checks for null, when the value cannot be null, except in case of bug.

By skipping code in case of null, you actually make things worse, in some ways. You prevent the code crashing at that point, but you just hide a bug, which will probably bite in some other way later. And it will probably do so in a way that's less obvious than a NullPointerException. Plus, unnecessary null tests obscure the true function of the code, making it harder to understand and harder to maintain.
 
chandrakant karale
Ranch Hand
Posts: 41
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
By skipping code in case of null, you actually make things worse, in some ways.


If null is not an allowed value of thatthing at that point, dont skip the code,throw an exception.

Plus, unnecessary null tests obscure the true function of the code, making it harder to understand and harder to maintain.



But then such checks, with proper excetptions thrown will prevent lot of problems. It may be harder to understand, but it will never to harder to maintain.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!