groovy
  1. groovy
  2. GROOVY-2879

Create @Identity annotation to autogenerate equals() / hashCode() / toString()

    Details

    • Type: New Feature New Feature
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.5.6
    • Fix Version/s: 1.8-beta-1
    • Component/s: None
    • Labels:
      None
    • Number of attachments :
      6

      Description

      The new @Bindable annotation is great. It (further) reduces the amount of boilerplate code required in Groovy beans. The next logical step is an @Identity annotation that can be applied to one or more attributes, telling Groovy that those properties define the identity / business key / whatever-you-want-to-call-it of the bean. The object's equals(), hashCode() and toString() would behave accordingly.

      May be related to GROOVY-27, but I'm not sure (was marked Won't Fix).

        Activity

        Hide
        blackdrag blackdrag added a comment -

        what code would you use for each method?

        Show
        blackdrag blackdrag added a comment - what code would you use for each method?
        Hide
        Daniel Gredler added a comment -

        Well, I doubt you want to add a dependency on Commons Lang, but I would probably make it:

        • equals(): the functional equivalent of using EqualsBuilder on the annotated properties [1]
        • hashCode(): the functional equivalent of using HashCodeBuilder on the annotated properties [2]
        • toString(): "UnqualifiedClassName[propName1=propValue1 propName2=propValue2]"

        [1] http://commons.apache.org/lang/api/org/apache/commons/lang/builder/EqualsBuilder.html
        [2] http://commons.apache.org/lang/api/org/apache/commons/lang/builder/HashCodeBuilder.html

        Show
        Daniel Gredler added a comment - Well, I doubt you want to add a dependency on Commons Lang, but I would probably make it: equals(): the functional equivalent of using EqualsBuilder on the annotated properties [1] hashCode(): the functional equivalent of using HashCodeBuilder on the annotated properties [2] toString(): "UnqualifiedClassName [propName1=propValue1 propName2=propValue2] " [1] http://commons.apache.org/lang/api/org/apache/commons/lang/builder/EqualsBuilder.html [2] http://commons.apache.org/lang/api/org/apache/commons/lang/builder/HashCodeBuilder.html
        Hide
        Robert Fischer added a comment -

        There is a huge issue with equals() in Java, which is equivalent to thie issue with compareTo() –

        http://enfranchisedmind.com/blog/2005/10/28/a-java-gotcha/
        http://enfranchisedmind.com/blog/2006/01/30/commons-lang-and-the-equalscompareto-debacle/

        That said, I'm all for this.

        Show
        Robert Fischer added a comment - There is a huge issue with equals() in Java, which is equivalent to thie issue with compareTo() – http://enfranchisedmind.com/blog/2005/10/28/a-java-gotcha/ http://enfranchisedmind.com/blog/2006/01/30/commons-lang-and-the-equalscompareto-debacle/ That said, I'm all for this.
        Hide
        Andres Almiray added a comment -

        @Immutable provides implementations for those methods, that being said it wont be that difficult to refactor the code.
        What about prototyping this AST in the Groovy Transforms module? http://groovy.codehaus.org/Groovy+Transforms

        Show
        Andres Almiray added a comment - @Immutable provides implementations for those methods, that being said it wont be that difficult to refactor the code. What about prototyping this AST in the Groovy Transforms module? http://groovy.codehaus.org/Groovy+Transforms
        Hide
        Paul King added a comment - - edited

        As well as the code in @Immutable which can be reused/refactored, a possible implementation exists here:
        http://github.com/kobo/kobo-commons/blob/master/src/main/groovy/org/jggug/kobo/commons/lang/EquivASTTransformation.groovy

        Show
        Paul King added a comment - - edited As well as the code in @Immutable which can be reused/refactored, a possible implementation exists here: http://github.com/kobo/kobo-commons/blob/master/src/main/groovy/org/jggug/kobo/commons/lang/EquivASTTransformation.groovy
        Hide
        Paul King added a comment -

        Also worth looking at is the @Data annotation from project Lombok:

        http://projectlombok.org/features/Data.html

        This provides additional annotations to customize the generated code, e.g.:

        @ToString(includeFieldNames=true)   // should pretty printing include field names
        @ToString(excludes="id")            // selecting fields for toString() and equals() etc.
        @EqualsAndHashCode(callSuper=true)  // include super for toString() and equals() etc.
        @Data(staticConstructor="of")       // create static factory method
        
        Show
        Paul King added a comment - Also worth looking at is the @Data annotation from project Lombok: http://projectlombok.org/features/Data.html This provides additional annotations to customize the generated code, e.g.: @ToString(includeFieldNames=true) // should pretty printing include field names @ToString(excludes="id") // selecting fields for toString() and equals() etc. @EqualsAndHashCode(callSuper=true) // include super for toString() and equals() etc. @Data(staticConstructor="of") // create static factory method
        Hide
        Paul King added a comment -

        See also @Mutable annotation in mailing list:
        http://groovy.markmail.org/thread/x7hyxmv3c37r4sa4

        Show
        Paul King added a comment - See also @Mutable annotation in mailing list: http://groovy.markmail.org/thread/x7hyxmv3c37r4sa4
        Hide
        Paul King added a comment -

        Attached patch from Paulo Poiati.

        Show
        Paul King added a comment - Attached patch from Paulo Poiati.
        Hide
        Paul King added a comment -

        Updated patch attached

        Show
        Paul King added a comment - Updated patch attached
        Hide
        Paul King added a comment - - edited

        Latest patch attached. Changes since last patch:

        groovy.lang.Immutable => groovy.transform.Immutable (groovy.lang.Immutable still there but deprecated)

        groovy.lang.Mutable => groovy.transform.Canonical

        removed map constructor from Canonical as we get that for free provided we have a noargs constructor (which the tuple constructor with default values provides)

        Show
        Paul King added a comment - - edited Latest patch attached. Changes since last patch: groovy.lang.Immutable => groovy.transform.Immutable (groovy.lang.Immutable still there but deprecated) groovy.lang.Mutable => groovy.transform.Canonical removed map constructor from Canonical as we get that for free provided we have a noargs constructor (which the tuple constructor with default values provides)
        Hide
        Paul King added a comment -

        Revised patch. Changes since last patch:
        Canonical remains but has also been split into its respective pieces which can be used separately:

        EqualsAndHashCode
        ToString
        TupleConstructor
        
        Show
        Paul King added a comment - Revised patch. Changes since last patch: Canonical remains but has also been split into its respective pieces which can be used separately: EqualsAndHashCode ToString TupleConstructor
        Hide
        Paul King added a comment -

        revised patch includes potential @AutoClone transform (not yet hooked into @Canonical)

        Show
        Paul King added a comment - revised patch includes potential @AutoClone transform (not yet hooked into @Canonical)
        Hide
        Paul King added a comment -

        Revised patch attached. Changes:

        • Adds AutoExternalize
        • Makes AutoClone configurable
        Show
        Paul King added a comment - Revised patch attached. Changes: Adds AutoExternalize Makes AutoClone configurable
        Hide
        Paul King added a comment -

        Patch applied - any help with further testing would be greatly appreciated.

        Show
        Paul King added a comment - Patch applied - any help with further testing would be greatly appreciated.
        Hide
        Daniel Gredler added a comment -

        I have a couple of concerns with the current implementation:

        1. The underlying annotations (EqualsAndHashCode, ToString, TupleConstructor) don't support a default-exclude mode; they're default-include (with the optional "excludes" attribute). I'd like to see an additional optional "includes" attribute that triggers a default-exclude mode. I think that in many scenarios the default-exclude approach would be more terse, less prone to bugs and more maintainable.

        2. The Canonical annotation and its underlying annotations (EqualsAndHashCode, ToString, TupleConstructor) are all class-level and require you to type in the attribute names a second time (for inclusion/exclusion), violating the DRY principle. It would be nice to have annotations that you can apply directly to the fields, triggering inclusion/exclusion.

        Show
        Daniel Gredler added a comment - I have a couple of concerns with the current implementation: 1. The underlying annotations (EqualsAndHashCode, ToString, TupleConstructor) don't support a default-exclude mode; they're default-include (with the optional "excludes" attribute). I'd like to see an additional optional "includes" attribute that triggers a default-exclude mode. I think that in many scenarios the default-exclude approach would be more terse, less prone to bugs and more maintainable. 2. The Canonical annotation and its underlying annotations (EqualsAndHashCode, ToString, TupleConstructor) are all class-level and require you to type in the attribute names a second time (for inclusion/exclusion), violating the DRY principle. It would be nice to have annotations that you can apply directly to the fields, triggering inclusion/exclusion.
        Hide
        Paul King added a comment -

        As a meta comment, typically comments added to closed issues aren't tracked very well (or at all after a while). It might be better to raise a new issue as a feature request.

        With respect to your suggestions...

        Having an includes is a reasonable suggestion but in my experience is relatively rare. Typically the things which make up an object's external identity (and hence might typically be in equals, hashCode etc.) are its properties. Anything which is secondary in nature (typically but not always) might be a private field. So the need for excludes is rather rare - and hence the use of includes would typically be lots more work. But perhaps you have a different use case which should be discussed.

        Regarding an attribute level annotation. Again another good idea but when we considered it initially, became a little bit messy. Would the annotation be @CanonicalProperty or just @Canonical, @ToString etc. in front of attributes? What it both @Immutable was in front of one property and @Canonical in front of another. Certainly a class with 20 properties soon became very non-DRY looking if each property had several annotation in front of it. Anyway, worthy of further discussion but I would suggest fleshing out some details so that discussion can progress further. Also, seeing what other people do, e.g. project Lombok helps make such arguments more persuasive.

        Show
        Paul King added a comment - As a meta comment, typically comments added to closed issues aren't tracked very well (or at all after a while). It might be better to raise a new issue as a feature request. With respect to your suggestions... Having an includes is a reasonable suggestion but in my experience is relatively rare. Typically the things which make up an object's external identity (and hence might typically be in equals, hashCode etc.) are its properties. Anything which is secondary in nature (typically but not always) might be a private field. So the need for excludes is rather rare - and hence the use of includes would typically be lots more work. But perhaps you have a different use case which should be discussed. Regarding an attribute level annotation. Again another good idea but when we considered it initially, became a little bit messy. Would the annotation be @CanonicalProperty or just @Canonical, @ToString etc. in front of attributes? What it both @Immutable was in front of one property and @Canonical in front of another. Certainly a class with 20 properties soon became very non-DRY looking if each property had several annotation in front of it. Anyway, worthy of further discussion but I would suggest fleshing out some details so that discussion can progress further. Also, seeing what other people do, e.g. project Lombok helps make such arguments more persuasive.
        Hide
        Pavlo Morgunyuk added a comment -

        Paul, the use case for 'includes' attribute usefulness in @EqualsAndHashCode is a JPA Entity, which shoud implement the equivalence on the natural key basis, and usually the natural key is one or two properties, while the entity can contain dozens of properties, and all of them have to be listed in 'excludes' at the moment...

        Show
        Pavlo Morgunyuk added a comment - Paul, the use case for 'includes' attribute usefulness in @EqualsAndHashCode is a JPA Entity, which shoud implement the equivalence on the natural key basis, and usually the natural key is one or two properties, while the entity can contain dozens of properties, and all of them have to be listed in 'excludes' at the moment...

          People

          • Assignee:
            Paul King
            Reporter:
            Daniel Gredler
          • Votes:
            1 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: