xvik / generics-resolver

Java generics runtime resolver
https://xvik.github.io/generics-resolver
MIT License
45 stars 9 forks source link

#7 added get generic context for field by only field name, and get ge… #8

Closed mikolajmitura closed 4 years ago

mikolajmitura commented 4 years ago

…neric context for type from root class by certain index.

codecov-io commented 4 years ago

Codecov Report

Merging #8 into master will increase coverage by 0.01%. The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master       #8      +/-   ##
============================================
+ Coverage     94.44%   94.45%   +0.01%     
- Complexity      873      876       +3     
============================================
  Files            43       43              
  Lines          1907     1913       +6     
  Branches        407      408       +1     
============================================
+ Hits           1801     1807       +6     
  Misses           24       24              
  Partials         82       82              
Flag Coverage Δ Complexity Δ
#linux 94.30% <100.00%> (+0.01%) 876.00 <3.00> (+3.00)
#windows 94.45% <100.00%> (+0.01%) 876.00 <3.00> (+3.00)
Impacted Files Coverage Δ Complexity Δ
...rics/resolver/context/AbstractGenericsContext.java 100.00% <100.00%> (ø) 38.00 <3.00> (+3.00)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b30105c...db67683. Read the comment docs.

xvik commented 4 years ago

First of all, thank you for your efforts! This is a very good pool request, but I can't accept it, sorry.

Generics-resolver purpose is to supplement reflection api in actually complex cases like generic variables tracking, working with generic types or other things that are hard to implement directly. It was done intentionally dependency-free and the addition of commons-lang is not an option.

If you look current api, you can see that there are no shortcut methods for building method, constructor or field context by string, only with actual Method, Constructor or Field objects. Of course, I was thinking about it but stopped myself because such things are very dangerous and hard to diagnose: it is very easy to forget switching context to correct type and try to lookup field or method by name on the incorrect type. Even more, fields with the same name may, for example, be present in different classes, and incorrect (by accident) resolution on the wrong type (like universal getGenericsInfo().getRootClass()) will lead to very hard to track errors. To shield from all of these problems, user is forced to do actual resolution himself and so have much fewer abilities to make an error. And even if he did, it would be much simpler to find a problem in user code rather then debugging 3rd party library.

From the other side, accepting exact object, like in field context GenericsContext fieldType(final Field field) allows me to know exactly what base type user means (because reflection object keeps declaration info) and so I can switch context to correct type automatically. This way I can shield user completely from wrong context usage (incorrect library usage).

Going forward, proposed improvements are extremely easy to implement:

public GenericsContext genericContextOf(final int position) {
        return inlyingType(genericType(position));
}

public GenericsContext fieldTypeByName(final String fieldName) {
        final Field field = resolveFieldByName(fieldName); // intentionally simplified        
        return fieldType(field);
}

And the use-cases covered by these methods are not very common. API is already complex and the addition of such options would make things only worse. To cover this exact case you will need only 5-min to create a small utility class in your code. But after that, you will know exactly what this code is doing and will completely control it. The library should only simplify complex things, not do everything possible.

Still, this (and case from the original issue) is quite interesting and deserves mention in documentation as an excellent usage example!