-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add (experimental) support of java.lang.Comparable #512
Comments
👤 maxxyme 🕗 Dec 20, 2012 at 16:14 UTC What steps will reproduce the problem? Implementing java.lang.Comparable for standard cases can be fastidious, because the calculation of the int return value might follow the same scheme as the ones used for the equals and hashCode (using each field, comparing, so on...) What is the expected output? What do you see instead? Provide an annotation to auto-generate the code for a basic appropriate compareTo() method in order to implement the java.lang.Comparable interface. |
👤 reinierz 🕗 Mar 11, 2013 at 23:33 UTC It's on our todo-list, but we want to take a somewhat wider view of this problem and introduce a more general annotation that indicates a complete strict relationship (so, it would also create compatible equals and hashCode methods and the name of the annotation will be a bit less unwieldy than @ EqualsAndHashCode). The biggest issue is ordering the fields, and being able to supply small comparator properties for each field, such as 'reverse direction on this'. We can't go overboard with that obviously (at that point, it's not really boilerplate anymore), and we also need to worry about what we do if any given field is not of a type we implicitly know how to order, and also isn't Comparable. More to the point perhaps to know if the field is comparable in the first place we need to resolve type on it and that's very difficult to do in a handler that needs to create methods. Not impossible (@ Delegate does it!) but it always leads to lots of complications. Generics also don't help. |
👤 Maaartinus 🕗 Dec 07, 2013 at 23:09 UTC I'd suggest to look for the uses of Guava's ComparisonChain. I'm afraid there's no easy solution for anything which is neither primitive nor Comparable; not even for arrays (IIRC my most recent Comparator ordered int[] lexicographically as unsigned with the exception of ordering shorter arrays first - surely not something you want to create a syntax for). Concerning the fields order, I believe that processing them as they appear in the source code is nearly sufficient except for you often want to skip a field (e.g. Maybe you should assume that all non-primitive fields are Comparable and get a compile-time error if they're not. The user could annotate the field with @ Comparator.UseComparator(MyComparator.class) to prevent this or to specify a special handling for Comparables. It gets a bit complicated concerning the number of annotations, but it's fairly regular and need no hacks and no resolution. Finally, I'd also add @ Comparator.Reverse and maybe @ Comparator.NullFirst and @ Comparator.NullLast. |
End of migration |
Proposal:
OPEN ISSUES:
|
Should equals and compareTo == 0 be identical? e.g. The Java API does not have this restriction AFAIK. Classes like HashMap and HashSet use equals() and hashCode() exclusively and classes like TreeMap and TreeSet only use compareTo. I can't find any class that uses both compareTo and equals. My classes adhere to (A) but not to (B) and I have never experienced any problems with this! |
My 2ec: Apart of that: +1. |
Just some random and partly conflicting ideas. Please let me know which ones you like.
Maybe not at all? Later, there may be additional options for
A big advantage of independent features is that someone could rather easily contribute them (generating methods and other magic is much harder than such modifications).
Ideally, combining IMHO Maybe there should be
I guess, you want to use the boolean to tell if the fields are equal or not; This could get cleaner with something like
I'd maybe replace
I'd suggest the latter, overridable with a configuration key and a class-wide option. When |
I couldn't help myself and tried to make my ideas into a complete proposal. Keeping I'd use Summary
Details
|
Two remarks:
|
There is something to be said about allowing both a nested class-level annotation and a field-level annotation for @Ordered.Conf. Some users like a clutter free class body and some like annotations to be as closely to the object it is related to (I prefer the latter). It should also be possible to specify @Ordered.Conf on a getter method instead of a field (in case some extra calculations are required on the value). |
Roel and I had a long talk on this and we did not reach any concensus. The major issues we just can't fix are:
We ran around in circles and didn't get anywhere with a design that felt anywhere even close to right. This proposal seems to be sinking for want of a name :/ |
What is the reason you want to include compareTo in @EqualsAndHashCode? Not every class that has a equals/hashCode needs a compareTo, so I would not have a problem with a separate @CompareTo annotation. A @FieldSetting annotation also doesn't seem a good idea to me. Do you really need a single exclusion/inclusion system? I agree that the goal should be to be as concise as possible, but separating the settings into several annotations (e.g. @EqualsAndHashCode.Field, @ToString.Field and @CompareTo.Field) would make the concept easier to understand to first-time users and lower the learning curve. |
We want to combine them because, IF want compareTo, you also want equals/hashCode, and almost always, you want the same batch of fields to define the behaviour of all those methods. We want one annotation; that doesn't mean the annotation always generates compareTo. That'd be a choice. But the same annotation powers both of it (you can choose between generating equals/hashCode/canEqual, or equals/hashCode/canEqual/compareTo. Those are the only 2 choices). Yes, we need a single exclusion/inclusion system, for all the reasons explained earlier. |
I understand why you would want to combine equals/hashCode and compareTo, but you concluded yourself that this would cause almost too much trouble with the nested field configuration. Maybe @CompareTo could borrow the field includes/excludes from @EqualsAndHashCode by adding an attribute like useEqualsAndHashCodeFields (with "true" being the default value). |
Here you have some examples of what I'm suggesting: Using nested annotations: @EqualsAndHashCode(
callSuper = true,
fields = {
@EqField(name = "id", doNotUseGetter = true),
@EqField(name = "extra", exclude = true),
}
)
@CompareTo(
fields = {
@CompField(name = "name", caseInsensitive = true)
}
)
public class Test {
private int id;
private String name;
private Object extra;
} and @EqualsAndHashCode(
exclude = {"extra"}
)
@CompareTo(
useEqualsAndHashCodeFields = false,
fields = {
@CompField(name = "id"),
@CompField(name = "extra", comparator = CompareExtra.class),
}
)
public class Test {
private int id;
private String name;
private Object extra;
} and/or using field-level annotations: @EqualsAndHashCode(callSuper = true)
@CompareTo()
public class Test {
@EqField(doNotUseGetter = true)
private int id;
@CompField(caseInsensitive = true)
private String name;
@EqField(exclude = true)
private Object extra;
} and @EqualsAndHashCode()
@CompareTo(useEqualsAndHashCodeFields = false)
public class Test {
@CompField()
private int id;
private String name;
@EqField(exclude = true)
@CompField(comparator = CompareExtra.class)
private Object extra;
} |
What if you provide just one annotation (EqualsAndHashCodeAndCompareTo) which implements a compareTo method based on the fields configuration of the annotation and this compareTo is just used in equals as well (return 0 == this.compareTo(other)) |
Some more ideas...
Then forget it and use 3 annotations:
Sure,
Maybe our includes/excludes thinking is a part of the problem. The two things are symmetrical, but only as long as you ignore configuration. When you exclude a field, then there's nothing more to configure. So maybe there should be nested
Also add Let me try:
|
Somewhat related. I dream of someday being able to write
and get a Java class that I would resemble a python Rough sketch
More to the pointWe can look at how other languages deal with this. In Python tuples are sorted lexicographically (first `bar, then baz above). If some other ordering is required, the general practice seems to be to define a new Comparator as required. As for the name of the new annotation, I offer |
Hi, I'd like to see this in future versions. Please add a milestone ;). |
We're picking this idea back up. For further discussion see https://github.com/rzwitserloot/lombok/wiki/FEATURE-IDEA:-Annotation-based-Include-Exclude-for-ToString,-EqualsAHC,-etc |
* added example for issue projectlombok#512 * fix removing of field annotations during delombok * cleanup code * added to changelog projectlombok#530 * added support for ImplicitResourceCloser extension point for @cleanup projectlombok#287 * master is now for: up from 2018.1 IntelliJ builds
Migrated from Google Code (issue 439)
The text was updated successfully, but these errors were encountered: