Prefer get getters over is getters in recorders #26895
Conversation
|
Thanks for your pull request! The title of your pull request does not follow our editorial rules. Could you have a look?
|
Christopher-Chianelli
changed the title
This comment has been minimized.
This comment has been minimized.
| List<Method> checkedMethodList = new ArrayList<>(); | ||
|
|
||
| for (Method method : methods) { | ||
| if (method.getName().startsWith("get")) { |
There was a problem hiding this comment.
Oh and thanks for given it a try :).
| Map<String, Method> getters = new HashMap<>(); | ||
| Map<String, Method> setters = new HashMap<>(); |
There was a problem hiding this comment.
I thought a bit more about it and I think you should have three maps here and add a isGetters map.
You would then at the end of the loop, remove everything from the isGetters map that has a key in getters (isGetters.keySet().removeAll(getters.keySet()) should do the trick). And then as a last step, you can add the isGetters content to getters.
That should simplify things a bit and would avoid going through the loop twice doing mostly the same thing.
There was a problem hiding this comment.
+1 on the map; looking at how getters is used, I think it would be more clear to change
Method get = getters.get(i);
to
Method get = getters.get(i);
if (get == null) {
get = isGetters.get(i); // If there is no "get" getter, use the "is" getter
}
(which in turn remove the need for removing the "get" getter keys from the "isGetters" map).
|
Can you squash both commits? Thanks! |
When both get getters and is getters exist, they can replace the original selected getter in PropertyUtils. This can creates inconsistencies, resulting in a field not being set, causing a NPE later down the line that is hard to diagnose. To remove inconsistencies, if there is both a get method and an is method, the get method is always preferred. This introduces consistent behavior for recorders. To accomplish this, a seperate map is used for isGetters in PropertyUtils. Using a seperate map removes the need to have a seperate loop for identifing the actual getter method. It also make the logic more clear (first try "get" getter, then try "is" getter).
Christopher-Chianelli
force-pushed
the
issue-26889
branch
from
bddd13d to
4fa92ff
Compare
|
Squashed |
gsmet
added
the
triage/waiting-for-ci
quarkus-bot
bot
removed
the
triage/waiting-for-ci
Fix #26899
When both get getters and is getters exist, they can
replace the original selected getter in PropertyUtils.
This can creates inconsistencies, resulting in a field
not being set, causing a NPE later down the line that is
hard to diagnose.
To remove inconsistencies, if there is both a get method
and an is method, the get method is always preferred.
This introduces consistent behavior for recorders.