3

I have a hashmap with key,value as Map<String,List<Trade>>. In the value object i.e. List<Trade> I have to compare each object. If any two of the object's property name "TradeType" is same then I have to remove those two from the list. I am trying to achieve as below. But I am getting "Array index out of bound exception" Also is there any better way to compare the same object inside list using streams??

Map<String,List<Trade>> tradeMap = new HashMap<>();
// Insert some data here
tradeMap.entrySet()
        .stream()
        .forEach(tradeList -> {
             List<Trade> tl = tradeList.getValue();
             String et = "";
         // Is there a better way to refactor this? 
             for(int i = 0;i <= tl.size();i++){
                   if(i == 0) {
                       et = tl.get(0).getTradeType();
                   }
                   else {
                       if(et.equals(tl.get(i).getTradeType())){
                           tl.remove(i);
                       }
                   }
              }
        });
1
  • 2
    Your index out of bounds error comes from here: i <= tl.size() ... you want <, not <= Commented Jul 22, 2018 at 4:26

4 Answers 4

2

Your description is not completely in sync with what your code does so I will provide a couple of solutions in which you can choose the one you're after.

First and foremost as for the IndexOutOfBoundsException you can solve it by changing the loop condition from i <= tl.size() to i < tl.size(). This is because the last item in a list is at index tl.size() - 1 as lists are 0 based.

To improve upon your current attempt you can do it as follows:

tradeMap.values()
        .stream()
        .filter(list -> list.size() > 1)
        .forEach(T::accept);

where accept is defined as:

private static void accept(List<Trade> list) {
      String et = list.get(0).getTradeType();
      list.subList(1, list.size()).removeIf(e -> e.getTradeType().equals(et));
}

and T should be substituted with the class containing the accept method.

The above code snippet only removes objects after the first element that are equal to the first element by trade type, which is what your example snippet is attempting to do. if however, you want distinct of all objects then one option would be to override equals and hashcode in the Trade class as follows:

@Override
public boolean equals(Object o) {
     if (this == o) return true;
     if (o == null || getClass() != o.getClass()) return false;
     Trade trade = (Trade) o;
     return Objects.equals(tradeType, trade.tradeType);
}

@Override
public int hashCode() {
    return Objects.hash(tradeType);
}

Then the accept method needs to be modified to become:

private static void accept(List<Trade> list) {
        List<Trade> distinct = list.stream()
                .distinct()
                .collect(Collectors.toList());

        list.clear(); // clear list
        list.addAll(distinct);  // repopulate with distinct objects by tradeType
}

or if you don't want to override equals and hashcode at all then you can use the toMap collector to get the distinct objects:

private static void accept(List<Trade> list) {
        Collection<Trade> distinct = list.stream()
                .collect(Collectors.toMap(Trade::getTradeType,
                        Function.identity(), (l, r) -> l, LinkedHashMap::new))
                .values();

        list.clear(); // clear list
        list.addAll(distinct);  // repopulate with distinct objects by tradeType    
}

if however, when you say:

"If any two of the object's property name "TradeType" is same then I have to remove those two from the list."

you actually want to remove all equal Trade objects by tradeType that have 2 or more occurrences then modify the accept method to be as follows:

private static void accept(List<Trade> list) {
       list.stream()
           .collect(Collectors.groupingBy(Trade::getTradeType))
           .values()
           .stream()
           .filter(l -> l.size() > 1)
           .map(l -> l.get(0))
           .forEach(t -> list.removeIf(trade -> trade.getTradeType().equals(t.getTradeType())));
}
Sign up to request clarification or add additional context in comments.

8 Comments

converting the List to intermediate data structure (LinkedHashMap) just to remove duplicates and getting back the refined List from it. Is it the best option? As I have your attention here, can you please comment on my answer below. I'm an admirer of your SO answers ( especially java-8). :)
The best option of the solutions provided is using the equals and hashcode approach for me. but as mentioned "if you don't want to override equals and hashcode" for whatever reason that is then using the toMap approach is another option .
@mark42inbound Thank you, I appreciate your kind words. when you say "can you please comment on my answer below." , regarding what exactly?
Thanks for replying! But defining the equality of an instance of a domain class based on the value of one of its properties might not be the criterion for its absolute equality. I mean what if the condition for equality of two list objects mentioned by OP is custom for this use case. The criterion for the actual equality might have different implementation altogether.
By my previous comment, I wanted an expert's view (as I had your attention here) about about my answer(to this question) ;) I have started to learn Java 8 recently and am answering questions on SO as a practice.
|
1
public void test(){
    Map<String, List<Trade>> data = new HashMap<>();
    List<Trade> list1 = Arrays.asList(new Trade("1"), new Trade("2"), new Trade("1"), new Trade("3"), new Trade("3"), new Trade("4"));
    List<Trade> list2 = Arrays.asList(new Trade("1"), new Trade("2"), new Trade("2"), new Trade("3"), new Trade("3"), new Trade("4"));

    data.put("a", list1);
    data.put("b", list2);

    Map<String, List<Trade>> resultMap = data.entrySet()
                                             .stream()
                                             .collect(Collectors.toMap(Entry::getKey, this::filterDuplicateListObjects));

    resultMap.forEach((key, value) -> System.out.println(key + ": " + value));
    // Don't forget to override the toString() method of Trade class.
}

public List<Trade> filterDuplicateListObjects(Entry<String, List<Trade>> entry){
    return entry.getValue()
                .stream()
                .filter(trade -> isDuplicate(trade, entry.getValue()))
                .collect(Collectors.toList());
}

public boolean isDuplicate(Trade trade, List<Trade> tradeList){
    return tradeList.stream()
            .filter(t -> !t.equals(trade))
            .noneMatch(t -> t.getTradeType().equals(trade.getTradeType()));
}

2 Comments

Just noticed your code performs different logic to the other answers but I can completely understand it as the OP mentioned "If any two of the object's property name "TradeType" is same then I have to remove those two from the list.". This is actually why in the first paragraph of my answer it mentions to the OP "Your description is not completely in sync with what your code does....". Anyhow, I've also updated my answer to take that piece of info into consideration. as an aside there are improvements you can make with your code but it's minor so we can let that pass :).
@Aominè, I just saw your last edit. That's some really good streaming ;)
1
Map<String,List<Trade>> tradeMap = new HashMap<>();
tradeMap.put("1",Arrays.asList(new Trade("A"),new Trade("B"),new Trade("A")));
tradeMap.put("2",Arrays.asList(new Trade("C"),new Trade("C"),new Trade("D")));

Map<String,Collection<Trade>> tradeMapNew = tradeMap.entrySet()
    .stream()
    .collect(Collectors.toMap(Entry::getKey,
                        e -> e.getValue().stream()                         //This is to remove the duplicates from the list.
                            .collect(Collectors.toMap(Trade::getTradeType,
                                        t->t,
                                        (t1,t2) -> t1,
                                        LinkedHashMap::new))
                            .values()));

Output:

{1=[A, B], 2=[C, D]}

Comments

0
Map<String, List<Trade>> tradeMap = new HashMap<>();

tradeMap.values()
    .stream()
    .forEach(trades -> trades.stream()
        .collect(Collectors.groupingBy(Trade::getType))
        .values()
        .stream()
        .filter(tradesByType -> tradesByType.size() > 1)
        .flatMap(Collection::stream)
        .forEach(trades::remove));

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.