Get duplicate count of objects using custom Equals

Question

I am creating a plugin in Java and have a Listing class where I have overriden the equals method like this:

   @Override
public boolean equals(Object listing) {
    if (listing == null)
        return false;
    if (listing instanceof Listing) {
        Listing l = (Listing) listing;
        return l.id.equals(this.id) &&
                l.getItems().stream().mapToInt(ItemStack::getAmount).sum() == this.getItems().stream().mapToInt(ItemStack::getAmount).sum() &&
                l.getItems().get(0).getType().equals(this.getItems().get(0).getType()) &&
                l.getSeller().equals(this.getSeller()) &&
                l.getPrice().equals(this.getPrice());
    }
    return false;
}

I have a cache of listings but I want to map listings to a count of duplicates with another equals method(Basicly the same comparator but without the id check). Currently I have made this, and it works

   public static Map<Listing, Long> getDuplicateCount(Collection<Listing> listings) {

    return listings.stream().collect(Collectors.groupingBy(e ->

            new Listing(-1,
                    e.getSeller(),
                    e.getItems(),
                    e.getCreatedTime(),
                    e.getPrice(),
                    e.isClaimed(),
                    e.isSold(),
                    e.getBuyer(),
                    e.getSoldDate(),
                    e.isCanceled()
            ), Collectors.counting()
    ));
}

but I want to keep one of the id's so the id isn't -1 for all of the returned entries(so if there is two duplicate entries but with id 5 and 10 it will just return one of them as key and the count 2 as value) any idea how to do that?

Answer 1

Maybe a bit easier would be to use a map with business id as a key:

Map<List, Integer> counts = new HashMap<>();
listings.forEach(lst -> counts.merge(getBusinessId(lst), 1, Integer::sum));

where getBusinessId is defined as follows (it defines a business id just for this comparison):

public List getBusinessId(Listing listing) {
    return asList(listing.getItems().stream().mapToInt(ItemStack::getAmount).sum(),
            listing.getItems().get(0).getType(),
            listing.getSeller(),
            listing.getPrice());
}

Looks easier to read than streams with groupingBy and such.

Answer 2

You can use something like

public static Map<Listing, Long> getDuplicateCount(Collection<Listing> listings) {
    return listings.stream().collect(
        Collectors.groupingBy(e ->
            new Listing(-1, e.getSeller(), e.getItems(), e.getCreatedTime(),
                e.getPrice(), e.isClaimed(), e.isSold(), e.getBuyer(),
                e.getSoldDate(), e.isCanceled()
            ),
            Collector.of(() -> new Object() {
                Listing oneOfThem;
                long count;
            },
            (o, l) -> { o.oneOfThem = l; o.count++; },
            (o1, o2) -> {
                o1.count += o2.count;
                if(o1.oneOfThem == null) o1.oneOfThem = o2.oneOfThem;
                return o1;
            })))
        .values().stream()
        .collect(Collectors.toMap(o -> o.oneOfThem, o -> o.count));
}

But it looks rather odd to me, that the equals method of Listing ignores several properties and is using the sum of a property of a contained list’s elements for determining equality. It looks like this equals method is already tailored to a specific grouping operation like this, which should not be used for a general equality rule.

Instead, you should define this special equality rule right in this operation:

public static Map<Listing, Long> getDuplicateCount(Collection<Listing> listings) {
    return listings.stream().collect(
        Collectors.groupingBy(l -> Arrays.asList(l.id,
                l.getItems().stream().mapToInt(ItemStack::getAmount).sum(),
                l.getItems().get(0).getType(), l.getSeller(), l.getPrice()),
            Collector.of(() -> new Object() {
                Listing oneOfThem;
                long count;
            },
            (o, l) -> { o.oneOfThem = l; o.count++; },
            (o1, o2) -> {
                o1.count += o2.count;
                if(o1.oneOfThem == null) o1.oneOfThem = o2.oneOfThem;
                return o1;
            })))
        .values().stream()
        .collect(Collectors.toMap(o -> o.oneOfThem, o -> o.count));
}

Now, the operation is entirely unrelated to how Listing implements equality, which allows the Listing class to provide a better, general purpose equality implementation. Or not to provide a custom equality at all, inheriting equals and hashCode from Object, which may be the right strategy for objects with a meaningful identity.

Answer 3

You can add this equalsWithoutId() method in your Listing class, which is just like equals() method, except the id field:

public class Listing {

    private int id;
    private String field1;
    private String field2;

    public boolean equalsWithoutId(Listing o) {
        if (this == o) return true;
        if (o == null) return false;
        return Objects.equal(field1, o.field1) &&
                Objects.equal(field2, o.field2);
    }

    //equals and hashcode here
}

Then your method will look like this:

public static Map<Listing, Long> getDuplicateCount(Collection<Listing> listings) {
    Map<Listing, Long> m = new LinkedHashMap<>();
    listings.forEach(listing -> m.entrySet().stream()
            .filter(e -> e.getKey().equalsWithoutId(listing))
            .findAny()
            .ifPresentOrElse(e -> e.setValue(e.getValue() + 1),
                    () -> m.put(listing, 1L)));
    return m;
}

Note that ifPresentOrElse was introduced in Java 9, so if you're on Java 8, you can use:

    listings.forEach(listing -> {
        Optional<Map.Entry<Listing, Long>> entry = m.entrySet().stream()
                .filter(e -> e.getKey().equalsWithoutId(listing))
                .findAny();
        if (entry.isPresent()) entry.get().setValue(entry.get().getValue() + 1);
        else m.put(listing, 1L);
    });

The performance will be of the order of O(n^2), but given your constraints with my other answer, I think this should work for you.

Answer 4

Not the best way, doesn't look very neat, but because there are no other answers, I'll post what came to my mind:

Summary

public class ParentListing {
  //all fields here except 'id'
  //constructors, getters, setters, toString, equals, hashcode
}

public class Listing extends ParentListing {

  private int id;

  public ParentListing getParent() {
    //not too happy creating a new object here, perhaps there is a better way
    return new ParentListing(getField1(), getField2());
  }

  //constructors, getters, setters, toString, equals, hashcode
}

and use it like this:

public static void main(String[] args) {
    List<Listing> list = List.of(
        new Listing(1, "f1", "f2"),
        new Listing(2, "f10", "f20"),
        new Listing(3, "f1", "f2"));

    //Following map maps listings (without id) to the list of IDs.
    //You can probably stop here and use it as it is. Or else see the next map.
    Map<ParentListing, Set<Integer>> collect = list.stream()
        .collect(Collectors.groupingBy(
            Listing::getParent,
            Collectors.mapping(Listing::getId, Collectors.toSet())));

    Map<Listing, Integer> map = collect.entrySet().stream()
        .map(e -> Map.entry(
            new Listing(e.getValue().stream().findFirst().get(), e.getKey()),
            e.getValue().size()))
        .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));

    System.out.println(map);
}

Output

{Listing{id=2, field1=f10, field2=f20} =1, Listing{id=1, field1=f1, field2=f2} =2}

Full code

@Getter
@Setter
public class ParentListing {
  private String field1;
  private String field2;

  public ParentListing(String field1, String field2) {
    this.field1 = field1;
    this.field2 = field2;
  }

  @Override
  public boolean equals(Object o) {
    if (this == o) return true;
    if (o == null || getClass() != o.getClass()) return false;
    ParentListing that = (ParentListing) o;
    return Objects.equal(field1, that.field1) &&
        Objects.equal(field2, that.field2);
  }

  @Override
  public int hashCode() {
    return Objects.hashCode(field1, field2);
  }

  @Override
  public String toString() {
    return "ParentListing{" +
        "field1='" + field1 + '\'' +
        ", field2='" + field2 + '\'' +
        '}';
  }
}

@Getter
@Setter
public class Listing extends ParentListing {

  private int id;

  public Listing(int id, String field1, String field2) {
    super(field1, field2);
    this.id = id;
  }

  public Listing(int id, ParentListing parentListing) {
    this(id, parentListing.getField1(), parentListing.getField2());
  }

  public ParentListing getParent() {
    return new ParentListing(getField1(), getField2());
  }

  @Override
  public boolean equals(Object o) {
    if (this == o) return true;
    if (o == null || getClass() != o.getClass()) return false;
    if (!super.equals(o)) return false;
    Listing listing = (Listing) o;
    return id == listing.id;
  }

  @Override
  public int hashCode() {
    return Objects.hashCode(super.hashCode(), id);
  }

  @Override
  public String toString() {
    return "Listing{" +
        "id=" + id +
        ", field1=" + getField1() +
        ", field2=" + getField2() +
        "} ";
  }
}