0
\$\begingroup\$

I have implemented a trim method which removes objects having empty properties.

This list has average size of 3 or so. Probably, the max length is 7 or 8.

I wonder which of these implementations is faster:

Implementation 1

    private void trim() {
       Iterator<Child> iterator = mList.iterator();
       while (iterator.hasNext()) {
           Child child = iterator.next();
           if (child.isEmpty()) {
               iterator.remove();
           }
       }
   }

Implementation 2

    private void trim() {
       for (Child child : mList) {
           if (child.isEmpty()) {
               mList.remove(child);
           }
       }
   }

Implementation 3

   private void trim() {
      for (int i = 0; i < mList.size(); i++) {
        IChild child = mList.get(i);
        if (child.isEmpty()){
            mList.remove(i);
        }
      }
   }
\$\endgroup\$
  • 1
    \$\begingroup\$ I think second implementation gonna throw ConcurrentModificationException. \$\endgroup\$ – Emre Aktürk Jun 19 '17 at 16:39
  • \$\begingroup\$ The first two implementations invalidate the iterator whilst it's in use; the third likely skips elements following those that are deleted. So I don't think any of them are even correct. Are you sure this is ready for review yet? \$\endgroup\$ – Toby Speight Jun 19 '17 at 16:47
  • \$\begingroup\$ I'm sorry but I have to close this question because it's clear that your code is untested. The only option that works properly is the first one. Also, you're question is essentially the same as this one: codereview.stackexchange.com/q/64011/31562 \$\endgroup\$ – Simon Forsberg Jun 19 '17 at 16:55
  • \$\begingroup\$ Also, if you're interested in measuring speed, you should look into benchmarking. In fact, one of the users around here wrote a very good Benchmarking library for Java 8 \$\endgroup\$ – Simon Forsberg Jun 19 '17 at 16:57
  • \$\begingroup\$ It was a prevision. You do your job, its ok :) \$\endgroup\$ – Emre Aktürk Jun 19 '17 at 16:57
1
\$\begingroup\$

Implementation 2 will in fact throw ConcurrentModificationException. What comes to Implementation 3, consider you have a list \$1,2,3,4\$, and the third implementation is about to remove \$2\$ and \$3\$. First, the index is 1 and \$2\$ is removed. Now \$3,4\$ is shifted one position to the left so that the entire list is \$1,3,4\$. At this point, the index is 1 and is incremented to 2. Clearly, \$3\$ is omitted and is not removed.

Basically, if removal is done in iteration, your only choice is to use Iterator explicitly.

Hope that helps.

\$\endgroup\$
  • \$\begingroup\$ Thanks for review. I do not want to take your time but if you interested, what would be the best practice or any suggestions? \$\endgroup\$ – Emre Aktürk Jun 19 '17 at 16:55
  • 1
    \$\begingroup\$ @EmreAktürk It really depends. Actually, you can get 3rd to work if you iterate backwards from larger indices towards smaller indices. \$\endgroup\$ – coderodde Jun 19 '17 at 17:02

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