Java: for loop is breaking before all iteration due to method call

Question

I am facing a bizarre problem. In the below code fragment, the 2nd for loop is breaking after one iteration, when I am calling userRoles.removeRole(strRole). There are 2 elements in the list. The first for loop is executing twice. But the 2nd one is executing only once. The said method call returns boolean. Could anyone please help me what's the wrong in my code?

        if(userRoles != null)
        {
            List<String> roles = userRoles.getRoles();
            String strUserName = userRoles.getUserName();

            for(String strRole: roles)
            {
                System.out.println("role : " + strRole);
            }
            //for(String strRole: roles)
            for(int count = 0; count < roles.size() ; count++)
            {
                String strRole = roles.get(count);
                System.out.println("role before check: " + strRole);
                if(ur.hasRoleForUser(strRole, strUserName))
                {
                    System.out.println("role after check: " + strRole);
                    userRoles.removeRole(strRole);
                }
            }

            System.out.println("role length: " + userRoles.getRoles().size());
            if(userRoles.getRoles().size() > 0)
            {
                ur.addUserRoles(userRoles);
            }
            blnSuccess = true;
        }

Why should it be strange? You are removing an item from the list you are enumerating. 2 - 1 = 1. `getRoles` evidently returns the reference to the same list `removeRole` shortens. — Margaret Bloom, Feb 22 '17 at 12:31
@MargaretBloom yes you are right ... I didn't notice it properly ... I thought it as new object, that doesn't refer the list of the object , from which is the item is being removed. Thank you for your reply. — NPException, Feb 22 '17 at 13:00

kamehl23 · Answer 1 · 2017-02-22T12:43:57.213

2

The loop breaks because you remove an element of the list that you are traversing (After the remove, the size of your list is 1, so count < roles.size() becomes false)

In the loop you should firstly collect the elements that you are going to remove after the loop

edited Feb 22 '17 at 12:43

answered Feb 22 '17 at 12:30

kamehl23

522
3
6

Using an iterator will throw `ConcurrentModificationException` as `removeRole` cannot possibly call the `remove` method of said iterator. – Margaret Bloom Feb 22 '17 at 12:38
You are right, ive corrected the answer. Thank you @Margaret Bloom – kamehl23 Feb 22 '17 at 12:44

score 1 · Answer 2 · answered Feb 22 '17 at 12:34

Your for loop evaluates count < roles.size() before each iteration.

Due to the fact that in your first iteration, you invoke userRoles.removeRole(strRole), in the next time the loop will evaluate roles.size() the returned value will be 1. Since 1 is not bigger than 1 (the value of count at that point) the loop will halt further iterations.

As @kamehl23 offered, you should use list iterators. Another recommendation will be not to modify the elements you are currently traversing. Will save you a lot of bugs time.

Good luck.

You can modify the items of an iterator as long as it's not a structural modification, otherwise `ConcurrentModificationException`. — Margaret Bloom, Feb 22 '17 at 12:40

score 1 · Answer 3 · answered Feb 22 '17 at 13:04

Thank you everybody for your answers. I didn't notice it properly ... I thought the roles list as new object, that doesn't refer the list of the object , from which each item is being removed. Here is my updated code.

        if(userRoles != null)
        {
            List<String> roles = userRoles.getRoles();
            String strUserName = userRoles.getUserName();

            for(int count = roles.size()-1; count >= 0 ; count--)
            {
                String strRole = roles.get(count);
                if(ur.hasRoleForUser(strRole, strUserName))
                {
                    userRoles.removeRole(strRole);
                }
            }

            if(userRoles.getRoles().size() > 0)
            {
                ur.addUserRoles(userRoles);
            }
            blnSuccess = true;
        }

Java: for loop is breaking before all iteration due to method call

3 Answers3