reorder REST services parameter converter#1358
Conversation
| final String strValue = (String) requestParameters.getString(f.getName()); | ||
|
|
||
| if (field.isAnnotationPresent(Parameter.class)) { | ||
| Class<? extends IStringConverter<?>> converter = field.getAnnotation( |
There was a problem hiding this comment.
So right here, the code gets the Class object and tries to see if it is one of the converters, or if it's "NoConverter." It does this for every single field, even though only a few have converters, which seems inefficient to me. However, it is important to do this before some of the other checks, because there are certain cases where the other checks would catch the type. I.E. passwordCoverter converts a string to another string, so that would get caught in the String check
There was a problem hiding this comment.
seems reasonable enough, I'd check for converter == null at the front of the if statement even though apparently it must always get set to "NoConverter" in practice just to add a little more protection from NPEs
| } | ||
|
|
||
| public Field getField() { | ||
| return super.getField(); |
There was a problem hiding this comment.
why bother to override the method if its just going to delegate with super?
| ','); | ||
| } | ||
|
|
||
| public Field getField() { |
There was a problem hiding this comment.
why bother to override the method if its just going to delegate with super?
cjw5db
left a comment
There was a problem hiding this comment.
Beginner mistake with the super methods. removed these and also added the null check in my last commit
No description provided.