I have a method that merges the elements of the collection, if the fields of these elements are suitable for certain conditions. And under certain conditions, some of the elements need to be removed on the go , but when using foreach cycles, I cannot use delete from the collection because I stumble upon a ConcurrentModificationException . And I have to go through the collection in a double cycle using a counter, and all the time pulling the methods get(...) .

The problem is that the remove(...) operation for an ArrayList very expensive since all the elements need to be shifted. And the operation get(...) expensive for LinkedList . Thus, it turns out that I absolutely need to get rid of either one operation or another.

And I got into an unpleasant fork: I take a LinkedList , and remove get() - I get a ConcurrentModificationException , I take an ArrayList because of the remove(...) method, I lose all the performance.

Please help me to get out of this situation, so that not to lose productivity.

Here is my code:

 @Override public Collection<Order> automaticDeals(List<Order> orders) { for (int i = 0; i < orders.size(); i++) { for (int j = 1 ;j < orders.size(); j++) { List<Order> forDelete = deal(orders.get(i), orders.get(j)); for (Order order : forDelete) { orders.remove(order); } } } return orders; } private List<Order> deal(Order fstOrder, Order scdOrder) { List<Order> ordersForDelete = new LinkedList<>(); if (dealIsImpossible(fstOrder, scdOrder)) { return ordersForDelete; } if (fstOrder.getVolume() < scdOrder.getVolume()) { int volume = scdOrder.getVolume() - fstOrder.getVolume(); scdOrder.setVolume(volume); ordersForDelete.add(fstOrder); } else if (fstOrder.getVolume() > scdOrder.getVolume()) { int volume = fstOrder.getVolume() - scdOrder.getVolume(); fstOrder.setVolume(volume); ordersForDelete.add(scdOrder); } else if (fstOrder.getVolume() == scdOrder.getVolume()) { ordersForDelete.add(fstOrder); ordersForDelete.add(scdOrder); } return ordersForDelete; }
  • You cannot do a remove operation in the collection you are iterating over. For this reason, you have either a ConcurrentModificationException (an error in the iterator) or a logical error in the code. When you delete an item, your orders.size () will change and you will skip some items. - iksuy
  • one
    I didn’t quite understand why to keep the difference getVolume () in one element and delete another. in my opinion, it is easier to create another array faster and add new elements there - Senior Pomidor

1 answer 1

as I understood, the distinct operation on a special field is needed. This can be done using java 8 stream api with the implementation of its predicate.

 public class Order { private int volume; private int sec; public Order(int volume) { this.volume = volume; } public int getVolume() { return volume; } @Override public String toString() { return "Order{" + "volume=" + volume + '}'; } } public static void main(String[] args) { List<Order> collect = new ArrayList<Order>() {{ add(new Order(4)); add(new Order(1)); add(new Order(1)); add(new Order(8)); add(new Order(1)); add(new Order(2)); }}; List<Order> res = collect.stream().filter(distinctByKey(f -> f.getVolume())).collect(Collectors.toList()); res.forEach(System.out::println); } public static <T> Predicate<T> distinctByKey(Function<? super T, ?> keyExtractor) { Map<Object,Boolean> seen = new ConcurrentHashMap<>(); return t -> seen.putIfAbsent(keyExtractor.apply(t), Boolean.TRUE) == null; }

result of execution:

 Order{volume=4} Order{volume=1} Order{volume=8} Order{volume=2}

Ps can use parallel() to speed up operations

inspired by the source