java-object-diff icon indicating copy to clipboard operation
java-object-diff copied to clipboard

PrimitiveArrayDiffer/Field properties

Open Wolfgang-Winter opened this issue 11 years ago • 10 comments

  • new PrimitiveArrayDiffer
  • allow using Field properties of bean properties

Wolfgang-Winter avatar Dec 08 '14 11:12 Wolfgang-Winter

Coverage Status

Coverage decreased (-2.37%) when pulling e3f07af21095a918d4c155f1e4b0d129c7aec70c on Wolfgang-Winter:master into 8fb29d5d387546c9e2578a6a1c08b598caa9aa4b on SQiShER:master.

coveralls avatar Dec 11 '14 18:12 coveralls

Hi Daniel, what do you think about that code? I use this version now in my Cibet framework and it works well. I plan a Cibet release mid January because another project needs the new functionality by then. Do you think a release 0.91.2 is possible until mid January?

Wolfgang

Wolfgang-Winter avatar Dec 13 '14 12:12 Wolfgang-Winter

Coverage Status

Coverage decreased (-2.43%) when pulling 562ad3d78aa52cbb3b0b81faa266ff17f2879c85 on Wolfgang-Winter:master into 8fb29d5d387546c9e2578a6a1c08b598caa9aa4b on SQiShER:master.

coveralls avatar Dec 13 '14 15:12 coveralls

Coverage Status

Coverage decreased (-2.41%) when pulling 9e28cf0316bf26a335cf94bed955b052df43f8a5 on Wolfgang-Winter:master into 8fb29d5d387546c9e2578a6a1c08b598caa9aa4b on SQiShER:master.

coveralls avatar Dec 13 '14 15:12 coveralls

Hi @Wolfgang-Winter,

thanks a lot for your PR! Sorry this is taking me so long. I'm currently pretty swamped. Functionality-wise your proposed code looks good, but architecturally there are some things I'd like to change before merging. I don't know when I'll get to it, though, because there is only so much time I have left besides my day-job, freelance work and spending some time with my family. So there are two options here:

  1. We both hope that I find the time to make those changes myself before mid of January, which isn't unlikely, but cannot be guaranteed; or
  2. I could give you some pointers of things I'd like to change and you handle the implementation (even though some of those things may go a little beyond what you need for your use-case)

What do you think?

SQiShER avatar Dec 15 '14 22:12 SQiShER

Hallo Daniel,

ich habe nicht den Überblick über deine Architektur, natürlich sollte das da rein passen. Ich weiß aber nicht, ob ich das noch schaffe, ich muss auch noch einiges in meinem Framework machen bis Mitte Januar und da Cibet ein privates Projekt ist, mache ich das in der Freizeit. Lass uns mal Anfang nächsten Jahres schauen, wo wir stehen, dann kann ich das besser abschätzen. Ich habe mir Mitte Januar für ein Release gesetzt, damit mein Team zwei Wochen Zeit hat sich einzuarbeiten. Im Februar bin ich nicht da und danach wäre es zu spät.

Gruß

Wolfgang

Von: Daniel Bechler [mailto:[email protected]] Gesendet: Montag, 15. Dezember 2014 23:06 An: SQiShER/java-object-diff Cc: Wolfgang-Winter Betreff: Re: [java-object-diff] PrimitiveArrayDiffer/Field properties (#123)

Hi @Wolfgang-Winter https://github.com/Wolfgang-Winter ,

thanks a lot for your PR! Sorry this is taking me so long. I'm currently pretty swamped. Functionality-wise your proposed code looks good, but architecturally there are some things I'd like to change before merging. I don't know when I'll get to it, though, because there is only so much time I have left besides my day-job, freelance work and spending some time with my family. So there are two options here:

  1. We both hope that I find the time to make those changes myself before mid of January, which isn't unlikely, but cannot be guaranteed; or
  2. I could give you some pointers of things I'd like to change and you handle the implementation (even though some of those things may go a little beyond what you need for your use-case)

What do you think?

— Reply to this email directly or view it on GitHub https://github.com/SQiShER/java-object-diff/pull/123#issuecomment-67075210 . https://github.com/notifications/beacon/AJovAPjMOAx7yW9NMGcTQaTobZUukUlCks5nX1LHgaJpZM4DFgj0.gif

Wolfgang-Winter avatar Dec 18 '14 20:12 Wolfgang-Winter

Hallo Daniel, ich wünsche dir ein frohes neues Jahr. Es fängt gut an denn ich habe jetzt Zeit, noch etwas in java-object-diff zu machen. Was möchtest du noch geändert haben?

Wolfgang-Winter avatar Jan 02 '15 10:01 Wolfgang-Winter

Hi Wolfgang,

Happy New Year to you too. Great to hear you have some time for java-object-diff! I'll add some comments to the lines of code I have questions or change requests about.

P.S.: I'll stick to English, because these conversations are usually a very good documentation for others. But feel free to keep on writing in German if you like and I'll do my best to provide enough context for others to follow in my answers.

SQiShER avatar Jan 02 '15 20:01 SQiShER

Regarding the PropertyAccessor: how about changing it to the following property resolution hierarchy:

  • public field, getter & setter
    • use getter for read access
    • use setter for write access
    • getAnnotations returns annotation from getter
  • public field, no getter, no setter
    • use field for read access
    • use field for write access
    • getAnnotations returns annotations from field
  • public field, no getter, setter present
    • use field for read access
    • use setter for write access
    • getAnnotations returns annotations from field
  • public field, getter, no setter
    • use getter for read access
    • use field for write access
    • getAnnotations returns annotation from getter
  • no public field, but getter & setter present
    • use getter for read access
    • use setter for write access
    • getAnnotations returns annotation from getter

This way public, non-static fields would automatically be treated as properties, yet it is possible to alter read and write access via getters and setters.

I wouldn't go as far as automatically including private fields though, since it would introduce a much higher risk of breaking client code. If you really need access to private and protected fields, it would be good to either make it opt-in via new configuration setting or just adding a second introspector.

SQiShER avatar Jan 02 '15 22:01 SQiShER

I fear this is not a solution for me. I am comparing JPA entities and JPA annotations can be applied to the field OR the getter method(). It is up to the developer. Therefore I must check existence of the annotations on the field (if existing) AND on the read method.

Normally entity beans should follow Java beans conventions but there are indeed cases where an entity has a property without having a corresponding field. Some JPA providers use byte code enhancement to modify the entity classes. I observed this in integration tests of object comparing and merging. I took that into account in the provided code.

I am sorry I cannot provide you with test cases that fit into your environment as I have no knowledge of TestNG, Groovy nor Spock. I use junit and arquillian in a complex integration environment.

Von: Daniel Bechler [mailto:[email protected]] Gesendet: Freitag, 2. Januar 2015 23:08 An: SQiShER/java-object-diff Cc: Wolfgang-Winter Betreff: Re: [java-object-diff] PrimitiveArrayDiffer/Field properties (#123)

Regarding the PropertyAccessor: how about changing it to the following property resolution hierarchy:

· public field, getter & setter

  • use getter for read access
  • use setter for write access
  • getAnnotations returns annotation from getter

· public field, no getter, no setter

  • use field for read access
  • use field for write access
  • getAnnotations returns annotations from field

· public field, no getter, setter present

  • use field for read access
  • use setter for write access
  • getAnnotations returns annotations from field

· public field, getter, no setter

  • use getter for read access
  • use field for write access
  • getAnnotations returns annotation from getter

· no public field, but getter & setter present

  • use getter for read access
  • use setter for write access
  • getAnnotations returns annotation from getter

This way public, non-static fields would automatically be treated as properties, yet it is possible to alter read and write access via getters and setters.

I wouldn't go as far as automatically including private fields though, since it would introduce a much higher risk of breaking client code. If you really need access to private and protected fields, it would be good to either make it opt-in via new configuration setting or just adding a second introspector.

— Reply to this email directly or view it on GitHub https://github.com/SQiShER/java-object-diff/pull/123#issuecomment-68567139 . https://github.com/notifications/beacon/AJovAG_1MIVNFTDdlUNPEPOcqcxLVGsTks5ndw4ngaJpZM4DFgj0.gif

Wolfgang-Winter avatar Jan 05 '15 18:01 Wolfgang-Winter