What am I doing wrong? Why does this iterator keep running?
default List<T> greedyAlgorithm() { List<T> lst = new ArrayList<>(); T element = selection().next(); while(selection().hasNext()) { if(feasibility(lst ,element)) { assign(lst, element); } else { element = selection().next(); } if(solution(lst)){ return lst; } } return null; }
The feasibility function checks if the element is viable, if yes assign the element to the list. Then the solution checks if this is the ‘algo’ solution and returns the list, else check the next element in the list.
Advertisement
Answer
next
should always be executed inside the loop.
If the feasibility
condition isn’t false
or solution
true, then it’ll keep running without advancing.
I suggest rearranging the loop so you unconditionally call next
.
There may also be a problem if selection
returns a new Iterator
every call.
Perhaps something like this:
default List<T> greedyAlgorithm() { List<T> lst = new ArrayList<>(); for ( Iterator<T> selection = selection(); selection.hasNext(); ) { T element = selection.next(); if (feasibility(lst ,element)) { assign(lst, element); if (solution(lst)) { return lst; } } } return null; }
If selection()
returned an Iterable
you could use a posh for loop. Generally returning a type such as Iterable
that (kind of) doesn’t change internal state is better than a type that does, such as Iterator
(although there is remove
so Iterable.iterator
isn’t entirely innocent).