Using HashMap Iterator

Question

I have two methods to iterate through a hash map containing a key(String) and a value(Arraylist) and add all the values from the Arraylists to a single Arraylist.

Method one did not work so I created Method two which fixed the issue but i'm not sure why method one did not work. Could someone please explain why method two works and method one does not?

Method 1

public ArrayList<Person> getPeopleList() 
{
    Iterator<ArrayList<Person>> iter = people.values().iterator();

    ArrayList<Person> allPersons = new ArrayList<>();
    while (iter.hasNext()) 
    {
        for (int i = 0; i < iter.next().size(); i++) 
        {
            allPersons.add(iter.next().get(i));
        }

    }
    return allPersons;
}

Method 2

public ArrayList<Person> getPeopleList() 
{
    Iterator<ArrayList<Person>> iter = people.values().iterator();

    ArrayList<Person> allPersons = new ArrayList<>();

    ArrayList<Person> persons;
    while (iter.hasNext()) 
    {
        persons = iter.next();
        for (Person p : persons) 
        {
            allPersons.add(p);
        }

    }
    return allPersons;
}

@AmitK Not quite, OP is asking about problem, not potential improvements for already working code. — Pshemo
– Pshemo, Commented Apr 18, 2017 at 16:05
Method 1 is massively broken, as it's going to be calling next() twice per inner iteration, when it shouldn't be calling iter.next() at all - you want a single call to iter.next() per outer iteration. — Jon Skeet
– Jon Skeet, Commented Apr 18, 2017 at 16:06
@AmitK "Trouble-shooting, debugging, or understanding code snippets ... is off-topic for this site." — Radiodef
– Radiodef, Commented Apr 18, 2017 at 16:06

JB Nizet · Accepted Answer · 2017-04-18 16:06:52Z

5

for (int i = 0; i < iter.next().size(); i++)

This calls next() at each iteration of your for loop. You need to call it once, and store the result in a variable. Note that List has an addAll() method, making this for loop useless.

With streams, your whole code can be reduced to

return map.values().stream().flatMap(List::stream).collect(Collectors.toList());

answered Apr 18, 2017 at 16:06

JB Nizet

694k94 gold badges1.3k silver badges1.3k bronze badges

Sign up to request clarification or add additional context in comments.

Collectives™ on Stack Overflow

Using HashMap Iterator

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related