I would like to know if it is alright to combine two comparators into a single comparator for two completely different classes. Objects are to be sorted alphabetically by a string type name
property which is present in both the classes which is why I created a single comparator.
/**
* Comparator to sort Book or Magazine alphabetically based on
* their String type name property.
*/
public class BookOrMagazineComparator implements Comparator<Object> {
@Override
public int compare(Object o1, Object o2) {
if (o1 != null && o2 != null) {
if (o1 instanceof Book && o2 instanceof Book) {
Book b1 = (Book) o1;
Book b2 = (Book) o2;
return b1.getName().compareTo(b2.getName());
} else if (o1 instanceof Magazine && o2 instanceof Magazine) {
Magazine m1 = (Magazine) o1;
Magazine m2 = (Magazine) o2;
return m1.getName().compareTo(m2.getName());
}
}
return 0;
}
}
Could there by any side effect if I use the above approach?
Edit: I would like to add that Book/Magazine are actually enums.
It's opinion based but I would call it bad practice as it breaks the rule that every class should serve only one purpose (AKA Single Responsibility principle).
Also it is probably broken because if you pass anything beside Magazine
and Book
(like InputStream
and URL
to give wild example) it returns 0
meaning they are equal. You gave up on the type-safety by implementing Comparator<Object>
which is shame.
As suggested in the comments it will be good idea to find common interface of the two classes you care about and implement comparator for that interface. In that case it will make perfect sense.
Suggestion: You say the classes are unrelated but they have something in common - the name
if you have NamedItem
interface which both Magazine
and Book
implement you can create Comparator<NamedItem>
and you are all good to go.
Note: You added the classes are enums. No problem with that. Enum can easily implement interface.
Could there by any side effect if I use the above approach?
Yes. Tight coupling. Your comparator class will have dependencies on each class it references. That will be a slight impediment to re-use, as it will not be able to exist without the classes it references.
As pointed out in another answer - a class should serve only one purpose. That is the single responsibility principle, and this approach is a slight violation of that principle.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With