CODEWITHSUNDEEP
javaobjectcomparable
V. Anoubis
V. Anoubis

Reputation: 11

Using a comparable on 3 different classes

I'm trying to implement a function that returns the maximum object of a given Comparable (generic) list.

I have 3 classes that I have implemented their compareTo method that returns the 1 if this is bigger than other, -1 if this is smaller than other, and 0 if they're equal.

Now my problem is with understanding with how do I work with a generic input COmparable list.

Here's the signature of my function, and the code I wrote so far (that refuses to work on me):

public static Comparable<?> getMax(List<Comparable<?>> ls) {
    LinkedList<Comparable<?>> tmpComp = new LinkedList<Comparable<?>>();
    for (Comparable<?> c : ls)
        tmpComp.add(c);
    Comparable<?> maxObj = tmpComp.get(0);
    for (Comparable<?> c : tmpComp)
        if (c.compareTo(maxObj) > 0)
            m = c;
    return m;
}

I'm writing a system that has users in it, and ads. Users and ads both classes that have "profit" field on them that all I do in my compareTo methods is to compare which of the two (this, or other) have more profit and then just returns the right value according to that. The 3rd class is compared via another field, which is an int as well, that indicates the level (int) of the Quest.

Also that if statement, specifically, gives me an error of the type "is not applicable for the arguments".

Any clues?

Thanks in advance!

Upvotes: 0

Views: 467

Answers (3)

lexicore
lexicore

Reputation: 43691

Your three classes must be comparable to each other. For this they will need to implement Comparable<SomeX> where SomeX is their lowest common superclass. In the worst case, SomeX is Object.

If this is the case, you can simply do:

ls.stream().max(Comparator.naturalOrder())

Alternatively, instead of forcing your classes to implement Comparable<...>, you could capture comparison semantics in a Comparator<...> and then do:

ls.stream().max(comparator)

Using a comparator is better for cases where the order is not really "natural" for the type or where there may be different orders. I think this is the case here since you actually compare instances of different types. It is hard to argue that some order is "natural" for these instances as they don't even belong to one type.

If you compare your instances based on some property they share (like int getProfit()), it would make sense creating a common interface like Profitable. Then you could do:

ls.stream().max(Comparator.comparintInt(Profitable::getProfit))

Note that if you compare on privitive types, you should use comparingInt/comparingLong/comparingDouble instead of comparing to avoid unnecessary boxing and unboxing.

If you for some reason can't create and implement a common interface like Profitable, you can still use comparingInt and likes. You'll just have a much uglier lambda:

ls.stream().max(Comparator.comparintInt(l -> {
    if (l instanceof Ad) { return ((Ad) l).getProfit(); }
    else if (l instanceof Ransom) { return ((Ransom) l).getProfit(); }
    // ...
    else { throw new IllegalArgumentException(...); }
}))

Upvotes: 0

Sean F
Sean F

Reputation: 4605

The answer by Mạnh Quyết Nguyễn is good. But it does not account for the situation where you have multiple potential types T, which appears to be your situation.

So in that situation, just wrap your various classes with a single class and use his solution.

If you have a User class and an Ad class, then create a wrapper like so:

class ProfitMaker implements Comparable<ProfitMaker> {
    User user;
    Ad ad;

    public int compare(ProfitMaker p) {
         //check my profit and compare with profit of p
    }
}

Use that class as the "T" when usign the getMax from Mạnh Quyết Nguyễn.

Alternatively, use an interface 

 interface ProfitMaker extends Comparable<ProfitMaker> {
      int getProfit();
 }

Make both your User and Ad classes implement that interface, and that use that interface as the "T" along with the getMax method from Mạnh Quyết Nguyễn.

Upvotes: 0

sprinter
sprinter

Reputation: 27976

Reading your comment, I suggest you redesign your model to be:

interface ProfitGenerating {
    double getProfit();
}

class User implements ProfitGenerating {
    ...
}

class Advert implements ProfitGenerating {
    ...
}

List<ProfitGenerating> profits = ...;

Optional<ProfitGenerating> maxProfit = profits.stream()
    .max(Comparator.comparingDouble(ProfitGenerating::getProfit));

Upvotes: 1

Related Questions