Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate object comparison implementation (implement Comparable<T>) #281

Closed
lombokissues opened this issue Jul 14, 2015 · 8 comments
Closed

Comments

@lombokissues
Copy link

Migrated from Google Code (issue 208)

@lombokissues
Copy link
Author

👤 rodion.moiseev   🕗 May 05, 2011 at 03:03 UTC

Automated generation of Comparable<T>﹟compareTo(T) would be a great addition to a @ Data-annotated class, or by itself using @ Comparable annotation.

Proposed specification (the intuitive part):

  • Class T marked with @ Comparable by default implements Comparable bound to T and implements compareTo(T), taking into account all non-inherited instance fields in the order of declaration.
  • Changing comparison order and/or excluding fields can be done by explicitly specifying comparable fields in the desired order using @ Comparable({"field1","field2", ...}). Although, not very refactoring-friendly.

Tricky parts:

  • Due to a generics restriction, class T who's super class S already implements compareTo(S) legally can only be comparable to type S, which means it cannot implement Comparable<T> or compareTo(T). If you can trick javac around the problem it would be great. When, delomboking, however, you'll need some ugly casting.
  • If super class is not comparable it could be useful to both, ignore it completely, or use available getters in the order declared. Although the latter could introduce weird bugs, so should not be recommended.
  • For non-comparable fields, they should be ignored with a warning. To hide the warning the user would have to either make all fields implement Comparable, or explicitly exclude those fields in the @ Comparable({"field1", "field2", ...}) annotation.

There are probably couple of more corner cases that make it difficult to implement this cleanly, but even implementing the bare minimum I believe would be very useful!

Regards

@lombokissues
Copy link
Author

👤 reinierz   🕗 Jun 06, 2011 at 18:21 UTC

Comparable is not the right name for this because it would clash with java.util.Comparable. The problem with a superclass that already has Comparable means we can never add this to @ Data directly, it'd have to be a separate annotation. I'm not particularly bothered by this problem if its a separate annotation (You'll get errors and the fix is: You can't create a hierarchy of comparables. This is basically already screwed beyond all recognition in java anyway. Lombok can't really fix this!)

I'm more concerned about interaction with superclasses in general. Just like it would make no sense at all to generate an equals() method involving only the fields of the superclass (though we do let you get away with that right now, but you have to be explicit about it) - it's much harder to figure out what to do here. Optimally we call on super's implementation of comparable to sort this out, but you can't have a stack of inherited compareTo methods.

For non-comparable fields, I'd far prefer an error. I'd find it rather strange that behaviour changes directly when you make changes in comparability of your fields' types.

I doubt we'll ever add this to lombok. It shouldn't be too difficult to generate a Compara_tor_ implementation, and you don't even need lombok shenanigans to do so - a simple annotation processor is enough.

We might even adopt such a tool into lombok if it already existed and sees some use.

@lombokissues
Copy link
Author

👤 rodion.moiseev   🕗 Jun 11, 2011 at 02:06 UTC

Thanks for the reply. With regard to naming, how about @ CompareTo (as in @ ToString)? I would agree with you for the most part with regard to interaction with super classes. If the super class already implements Comparable it's probably for the best to prohibit the usage of @ CompareTo annotation in the subclass. Effective Java also recommends using composition over inheritance in such case.

For non-comparable fields, you're right, an error is probably the best.

I don't know exactly the kind of vision that drives this project, but I believe that generating compareTo has all of the advantages implementing getters, setters, equals, etc has. I don't know of any simple annotation processor to do the job, and I doubt many other Java do. Implementing one myself is not an easy task, as you say. So in the end I will end up using source-code generation or CompareToBuilder(in apache commons), which has all the disadvantages of source-code generation of getters, setters, equals, etc.

The cool thing about Lombok is it's intuitive interface: you add @ ToString and it gives you toString(), you add @ Getter and it gives you a getter. I feel it's only natural to add @ CompareTo and see a compareTo() in your class. Let me know what you think.

@lombokissues
Copy link
Author

👤 reinierz   🕗 Jun 11, 2011 at 05:00 UTC

Checking superclass is not really feasible; running processors at a point in time where we can resolve (i.e. ask questions such as "what methods are in my superclass") is very tricky. So far only @ Delegate and val get this treatment because they can't work without it.

I agree but this is so far down the list of 'want to haves' that I doubt we'll ever get to it. Now a patch of some sort, that would make it in, no problem.

@lombokissues
Copy link
Author

👤 rodion.moiseev   🕗 Jun 11, 2011 at 05:30 UTC

Thanks. I see, seems like removing any support for super-classes is probably the best option for compareTo() support.

From my personal experience with Lombok, as a user of Lombok in production environment, it would definitely help having compareTo(). We have lots of immutable data objects, and introducing Lombok has removed ALL code from class definitions, except compareTo()s which as still dangling around (leaves me wondering: why, oh why?). So in my personal opinion, support for compareTo() should be as natural as the one for other features (personally, I would even prefer it to 'val'). I don't understand why would you treat it as a 'want to have'.

Anyway, I think Lombok is one of the best things that happened to Java, and I am very much looking forward to seeing more boiler-plate disappearing.

PS. I didn't know there is support for @ Delegate, I'll definitely check it out.

@lombokissues
Copy link
Author

End of migration

@douglassparker
Copy link

I'm not sure why you think this would be difficult. The attributes of @CompareTo would be the same as for @EqualsAndHashCode. Use Apache Commons Lang CompareToBuilder to implement.

@CompareTo(callSuper=true,  include={"field1", "field2", "field3"}
class Foo {
}

//generates
class Foo implements Comparable<Foo> {
...
    public int compareTo(Foo foo) {
        return new CompareToBuilder()
              .appendSuper(super.compareTo(foo)
             .append(this.field1, foo.field1)
             .append(this.field2, myClass.field2)
             .append(this.field3, myClass.field3)
             .toComparison();
    }
}

Actually, it would have to be a little more complicated, because you would have to allow for sorting in descending order for each field. I agree that this should not be allowed unless all selected fields are themselves Comparable.

It also might be nice to allow a custom comparator got complex cases, e.g,

@CompareTo(comparator=FooComparator.class)

This would be a very handy feature and I urge you to reconsider.

Kudos! Lombok is a fantastic framework.

@Maaartinus
Copy link
Contributor

This issue is closed, but there's a newer discussion (but no progress) in #512.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants