Comparing String elements in an ArrayList alphabetically

Question

In this method I am comparing String elements alphabetically. In other words, if an element in a ArrayList is alphabetically bigger than the minimum String (parameter in the method) and is smaller than the maximum String (also parameter) then this String element should be removed from the ArrayList.

However, after running the method there are no Strings returned, not even those which should stay after the method call. In addition, when the IF condition is not satisfied, it printed out "Error" without returning the list.

import java.util.ArrayList;

public class Main {

    public static ArrayList<String> removeInRange(ArrayList<String> list, String beginning, String ending)
    {

        for (int i = 0; i<list.size(); i++)
        {
            if (list.get(i).compareTo(beginning)> 0 && list.get(i).compareTo(ending)< 0)
            {
                list.remove(list.get(i));
            }

            else {

                System.out.println("Error");
            }
        } return list;
    }

    public static void main(String[] args) {

        ArrayList<String> list = new ArrayList<>();
        list.add("h");
        list.add("e");
        list.add("x");

        removeInRange(list, "a", "k");

    }
}

The somewhat broken removal logic aside, whats your problem? There is no place your "...but the list is still not returned." could apply to in the code your show. — Durandal, Feb 09 '16 at 16:07

score 0 · Answer 1 · answered Feb 09 '16 at 16:08

During first iteration, h is removed from the list. So, e is moved to the first location. So e is no more compared. Use iterator instead.

Iterator<String> iterator1 = list.iterator();
while(iterator1.hasNext()){
    String i = iterator1.next();    
    if (i.compareTo(beginning)> 0 && i.compareTo(ending)< 0)
    {
        iterator1.remove();
    }
    else {
        System.out.println("Error");
    }
} 
return list;

score 0 · Answer 2 · answered Feb 09 '16 at 16:12

A few things I would point out here.

First, I would advise against editing an object while you are iterating over it, will just cause confusion.

Second, based on that point, it would be easier if you instead add to a new list the ones to keep, then return a new list. But if you want to modify the existing list, that is also possible.

public static ArrayList<String> removeInRange(ArrayList<String> list, String beginning, String ending) {
  ArrayList<String> matching = new ArrayList<String>();
  for (int i = 0; i<list.size(); i++) {
    if (list.get(i).compareTo(beginning)> 0 && list.get(i).compareTo(ending)< 0)
    {
        matching.add(list.get(i));
    }
    else {
        System.out.println("Error");
    }
  }
  list.removeAll(matching);

  return list;
}

Or just flip the condition and `return matching` if you don't want to edit the list — OneCricketeer, Feb 09 '16 at 16:15
Right, that's what I was thinking in my second point, but OP may want the original list edited for some reason, so kept with that logic. — GoGoCarl, Feb 09 '16 at 16:18

OneCricketeer · Answer 3 · 2016-02-09T16:32:58.133

If you are using Java 8.

Note, this won't print "Error", which, personally, doesn't make much sense because it is not an error, you are just ignoring values.

public static void main(String[] args) {
    ArrayList<String> list = new ArrayList<>();
    list.add("h");
    list.add("e");
    list.add("x");

    List<String> filtered= removeInRange(list, "a", "k");
    System.out.println(filtered);

}

private static List<String> removeInRange(ArrayList<String> list, String low, String high) {
     return list.stream().filter(s -> s.compareTo(low) <= 0 || s.compareTo(high) >= 0).collect(Collectors.toList());
}

Comparing String elements in an ArrayList alphabetically

3 Answers3