Avoiding label in a loop to sort links

How can I refactor my code to remove the label and the need for it? I am aware that a loop with label is "forbidden". But I can not find a way to rewrite this without the label.

private List sortLinks(SegmentType s, Set LinkSet) {
        List LinkList = new LinkedList();

        String dep = s.getDep().toString();
        mainLoop: for (int index = 0; !LinkSet.isEmpty(); index++) {

            for (Iterator iterator = LinkSet.iterator(); iterator.hasNext(); ) {
                myList link = iterator.next();
                if (link.getLegDep().toString().equals(dep)) {
                    iterator.remove();
                    link.setLine(s.getLineCode());
                    link.setNb(s.getNb());
                    link.setSuff(s.getSuff());
                    link.setIndex(index);
                    linkList.add(link);

                    dep = link.getDest().toString();
                    continue mainLoop;
                }
            }

            return Collections.emptyList();
        }
        return linkList;
    }

In general, labelled code is uncommon, but 'forbidden' is a bit harsh. Break, and Continue have better characteristics than GoTo, and should not be 'painted with the same brush'.

The logic in your code is convoluted though.... you are 'searching' the input set, and ensuring that all members of the input set match a condition. As you search, if the member matches, you remove the member. If one of the members does not match, you immediately return an empty result.

Note, that if a 'middle' member fails to match, you have already removed the first members, and yet you return an empty set.

Your logic could be significantly simplified if you extracted part of your method as a 'helper' function:

private myList searchAndRemove(String dep, Set candidates) {
    for (Iterator iterator = candidates.iterator(); iterator.hasNext(); ) {
        myList link = iterator.next();
        if (link.getLegDep().toString().equals(dep)) {
            iterator.remove();
            return link;
        }
    }
    return null;
}

private List sortLinks(SegmentType s, Set LinkSet) {

    List matched = new LinkedList();

    String dep = s.getDep().toString();
    int index = 0;
    while (!LinkSet.isEmpty()) {
        myList link = searchAndRemove(dep, LinkSet);
        if (link == null) {
            return Collections.emptyList();
        }
        matched.add(link);
        link.setLine(s.getLineCode());
        link.setNb(s.getNb());
        link.setSuff(s.getSuff());
        link.setIndex(index++);

        dep = link.getDest().toString();
    }

    return matched;
}

In addition to the above, note that your class names and variable names are really, really bad....

• myList is a class, and should be CapitalCamelCase naming style, and have a different name.

• LinkList is a variable name, and should be lowerCamelCase, and have a name like matchedSegments

• LinkSet is also a variable name, and should be lowerCamelCase.

In general, with the name-pollution of having a class with 'Link' in the name (probably because it's not a horrible name', but it conflicts with LinkedList, so try to avoid Objects with Link as the name.

Additionally, note that your index variable was not an index in to the source data, but an index of the output segment. Including it as part of the original 'for' loop implied that it was used as an index in to that. The reality is that it is independent. I have edited it to make that clear.

StackExchange Code Review Q#68990, answer score: 4