m-m-m / util

Mature Modular Meta-Framework
http://m-m-m.sourceforge.net
Apache License 2.0
10 stars 5 forks source link

Field tracking for PojoPropertyDescriptors #132

Closed maybeec closed 9 years ago

maybeec commented 9 years ago

I implemented the traceability of Fields for PojoPropertyDescriptors if possible. Thus a PojoPropertyDescriptor now has a reference to its representing Field. If the PojoPropertyDescriptor cannot be traced to a Field getField() will return null.

hohwille commented 9 years ago

Thanks for your work. Have you seen that you can already get the field via PojoPropertyAccessor.getAccessibleObject() ? Further your change will currently break GWT compatibility so I would need to do some rework before. First of all I consider your feedback and suggestion such that the API of property introspection is either too complex to find the right way to get what you want (so documentation at least needs to be improved) or too complex to use at all. I am aware that it is complex but the underlying reflection is actually complex and so are the possibilities that you can take out of these. I will think about it but I need a little more time. Thanks already so far...

hohwille commented 9 years ago

Could you give feedback if also getting the GET accessor and calling getAccessibleObject() and casting to Field would do it? What is your usecase? getAccessibleObject() was provided so you can read stuff like annotations while abstracting if you do field or method introspection. If you want to rely on the fields and work with them instead isnt the entire property introspection overkill then?

hohwille commented 9 years ago

I am currently more thinking about convenience methods in PojoPropertyDescriptor to get e.g. the accesible from the getter in a single method call (or null if no getter). BTW: After 7.0.0 is released this will become a maintenance release stream while 8.0.0 will require java8 and allow to use default methods, etc. However, in such special cases adding methods to interfaces that probably nobody outside will implement should not be an issue (we also always provide an abstract base implementation and documentation strictly suggests to extend abstract base implementation rather than implementing interface directly).

hohwille commented 9 years ago

Looking through your changes again: are you using "private field + public method" introspection? If you want to get both method access and filed access then this will actually be a valid point for extending...

maybeec commented 9 years ago

Have you seen that you can already get the field via PojoPropertyAccessor.getAccessibleObject() ?

So initially I try to get all fields by the privateFieldIntrospection. Then I could not check whether the property can be accessed in an object of the current target class. Thus I did not try to get the object in this situation.

Could you give feedback if also getting the GET accessor and calling getAccessibleObject() and casting to Field would do it?

Next according to your advice, I started to work with publicMethodIntrospection, which returns the intended results. Now calling getAccessibleObject() I got a Method object. Thus I did not further investigate this.

Further your change will currently break GWT compatibility so I would need to do some rework before.

Sry for that, I have not been aware of this. Is there any possibility to cover this in a test? Otherwise, it might not be possible to check such implicit contraints for externals ;)

What is your usecase?

I try to get all declared fields of the target type and its super types, which are accessible in the target type using an accessible setter method and an accessible getter method. This might be interesting for generation of test data builder. The generated builder e.g. should provide builder methods for all fields of a type to be built, which are accessible using getters and setters. This preserves from generation of unnecessary builder methods for properties, which might only be a delegation to a setter of some referenced type.

BTW: After 7.0.0 is released this will become a maintenance release stream while 8.0.0 will require java8 and allow to use default methods, etc.

Interesting, but maybe a little bit risky when more industrial projects might make use of your library, which might also have some further feature requests. I think you know by heart, that industries do not use the latest versions of technology like Java 8 :)

Looking through your changes again: are you using "private field + public method" introspection? If you want to get both method access and filed access then this will actually be a valid point for extending...

:+1: YES - see use case description above

hohwille commented 9 years ago
  1. I am working on a testcase for GWT compatibility but this has to be placed in mmm-util-gwt and can not go to mmm-util-core as I did not want to overload util-core with GWT super-sourced code for people that do not need GWT support.
  2. I will then try to integrate your pull request but will need some rework.
  3. For java7 vs. java8 you got me wrong. We will still keep support for java7 but in 7.x.y. versions where bugs will be fixed and then merged to 8.x.y. branch. However, there is a lot that can be tidied up in the future with java8 and we want to get started with that in the next future.
maybeec commented 9 years ago

I am working on a testcase for GWT compatibility but this has to be placed in mmm-util-gwt and can not go to mmm-util-core as I did not want to overload util-core with GWT super-sourced code for people that do not need GWT support.

I think having at least one test in mmm-util-gwt would be ok as long as travis will also run the test cases of mmm-util-gwt. Thus you and the externals will get the test run of interest for any further pull-request.

hohwille commented 9 years ago

Right but travis is not triggered again if the master changed after the pull-request has been made. So now the info is still "all is well" but this would actually be wrong if I click merge pull request...

hohwille commented 9 years ago

I will try integrate your patch but will need to do some rework.

This might be interesting for generation of test data builder. The generated builder e.g. should provide builder methods for all fields of a type to be built, which are accessible using getters and setters. This preserves from generation of unnecessary builder methods for properties, which might only be a delegation to a setter of some referenced type.

You should be aware that the matching of field and methods is just by name. People should not but can write strange code like

public String getFoo() { return this.bar; } public Long getBar() { return this.foo; }

What you actually want to figure out is if a getter is actually representing a calculated or shadowed property or if it is really non-transient giving access to a real backed field. This can be done by convention and might work in reasonable cases but might cause unexpected results in special cases.

The actual pain is that Java does not offer a property concept. All the reflection and introspection support I provide would all not be necessary then and our lives would be a lot easier. Also the performance would be a lot better if you would not have to re-implement what the compiler already did on reflection level. Even more pain is caused in the area of data-binding. There could be such elegant ways to bind a property to something (e.g. a UI widget) including validation support via BV and more if only java would introduce real properties so there is an easy API to iterate the properties of a bean allowing to get their name and type as well as reading and writing to them for a given instance all in combination of type safe referencing a property in the code. After generics and lambdas this is still the missing link to make java a reasonable and great language. Sorry to say so...

hohwille commented 9 years ago

@may-bee: As I changed and reworked quite a bit you could give it a test from the latest master if possible.