3
\$\begingroup\$

I have a List, and I have to delete one item from the list when I find it. I am using the code below to do that:

Iterator<MyClass> iter = flist.iterator();              
while (iter.hasNext()) {
    Long deletedFiId = iter.next().getId(); 
    if (deletedFiId.equals(fiId)) { 
         iter.remove();
         break;
     }
}

One of my co-workers suggested I should hold the object to be removed and remove it once we get out of the for loop, like this:

MyClass deletedFiId = null;
for (MyClass dto: flist) {
  if (dto.getId().equals(fiId)) {
    deletedFiId = dto;
    break;
  }
if (deletedFiId != null)
  fiList.remove(deletedFiId);

Which one of the above is a good-to-have in the code?

\$\endgroup\$
  • 1
    \$\begingroup\$ Calling iter.next() on an Iterator<Object> should return an Object. How do you call .getId() on an Object? \$\endgroup\$ – 200_success Oct 16 '13 at 20:20
  • \$\begingroup\$ Don't care if there's a third, better way? \$\endgroup\$ – Mathieu Guindon Oct 16 '13 at 20:55
  • \$\begingroup\$ @200_success: I did change, the then confusing datatype. \$\endgroup\$ – gsb Oct 16 '13 at 21:16
  • \$\begingroup\$ @retailcoder: I would definitely like to know a better way, other than the above two. Is there any? \$\endgroup\$ – gsb Oct 16 '13 at 21:17
  • \$\begingroup\$ I don't know really, it's just the way you've worded your question looks like it's narrowing the kind of code reviews you'll get to "Go with A because xyz" or "Go with B because xyz". It's not forbidden, but it's not the best way to ask for a review and get the best out of the reviewers imho. \$\endgroup\$ – Mathieu Guindon Oct 16 '13 at 22:05
3
\$\begingroup\$

The two methods are functionally identical. The first method is faster to execute. The first method is clearer for someone who doesn't know the code.

Unless there's an "else" you need (that is, logic to execute if the item can't be found), I can't think of a reason to prefer the second approach.

First approach wins.

\$\endgroup\$
2
\$\begingroup\$
  1. The first one uses equals while the second one uses == for the comparison. They are not the same.

  2. In the second snippet fi could be renamed to deletableItem or something more meaningful and easier to read. (How to abbreviate variable names on Programmers.SE.)

  3. iter.remove in first snippet uses equals of your ID class while the second snippet uses fiList.remove which uses fi.equals() (javadoc) It might produce different result lists depending on the implementation of your equals method.

\$\endgroup\$
  • \$\begingroup\$ I changed the second snippet to use .equals() and have same name as the first snippet. \$\endgroup\$ – gsb Oct 16 '13 at 21:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.