Ivan Dankov
Ivan Dankov

Reputation: 15

Java: Creating a getter function which returns an iterator from an array list

I have a class named User I have a class named UserGroup(an arrayList of users) which contains the following:

variables:

private ArrayList<User> users;
private Iterator<User> userIterator;

constructor:

public UserGroup(){
    this.users = new ArrayList<>();
}

I'm trying to get the following to work:

public Iterator<User> getUserIterator(){
    userIterator = users.iterator();
    return userIterator;
}

And I'm hoping that it returns an iterator which I can use to iterate through a UserGroup by using it in the following context, for example:

while(groupOfUsers.getUserIterator().hasNext()) {
        System.out.println(groupOfUsers.getUserIterator().next());
    }

This is resulting in some terrifying infinite loop, and I honestly don't even understand what my getter method is returning at this point. (java.util.ArrayList$Itr@74a14482)

Any help in getting my head around how I would actually go about creating a getter that works in returning an iterator that I could use to iterate over any UserGroup would be much appreciated.

Upvotes: 1

Views: 1500

Answers (3)

slipperyseal
slipperyseal

Reputation: 2778

Don't store the Iterator as a member variable. An Iterator is something you create every time you need to iterate the loop, and you use that local instance to do so. Multiple Iterators on the same list could be in use at the same time. So there's no need to store it in the object.

remove this line..

private Iterator<User> userIterator; // not needed. bad.

then change your getUserIterator() method to this...

public Iterator<User> getUserIterator(){
   return users.iterator();
}

Then use the returned instance like this. But don't call getUserIterator again as it will return you a new one, at the start of the list. (This was the cause of the infinite loop)

Iterator<User> iterator = groupOfUsers.getUserIterator();
while(iterator.hasNext()) {
    System.out.println(iterator.next());
}

As an advanced step, you can use the enhanced for loop like this..

for (User user : groupOfUsers) {
     System.out.println(user);
}

but only if you make your groupOfUsers class implement the Iterable interface. And change the name of the getUserIterator method to iterator()

public class GroupOfUsers implements Iterable<User> {
    private ArrayList<User> users;

    public Iterator<User> iterator(){
        return users.iterator();
    }

    // etc..

Upvotes: 2

xiaofeng.li
xiaofeng.li

Reputation: 8587

Your getter is fine. The problem is with how you use the returned iterator.

while(groupOfUsers.getUserIterator().hasNext()) 

The above code will try to get a new iterator in the beginning of every iteration, so if the group of users is not empty, the loop condition will always be true, and you have an infinite loop.

Do this instead

Iterator<User> iter = groupOfUsers.getUserIterator();
while (iter.hasNext()) System.out.println(iter.next());

Or if you just want to do something with each user in the group and are using Java 8. You can provide a foreach method taking a Consumer<User> argument. Iterator may allow the client to remove an element from the list, and sometimes you don't want that to happen.

public void foreachUser(Consumer<? super User> consumer) {
    users.forEach(consumer);
}

groupOfUsers.foreachUser(System.out::println);

Upvotes: 0

Andy Turner
Andy Turner

Reputation: 140514

Assign the iterator to a variable first:

Iterator<User> it = groupOfUsers.getUserIterator();
while(it.hasNext()) {
    System.out.println(it.next());
}

Otherwise, you are creating a new iterator (pointing to the start of the list) each time you call groupOfUsers.getUserIterator() - and provided the list isn't empty, groupOfUsers.getUserIterator().hasNext() will then always return true.

Upvotes: 1

Related Questions