-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor changes #880
Refactor changes #880
Conversation
@@ -1806,6 +1801,15 @@ && isValidMethodName(method.getName())) { | |||
} | |||
} | |||
|
|||
private boolean isValidMethod(int modifiers,Method method) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a JavaDoc comment to this new method.
@@ -113,24 +113,38 @@ public static JSONObject toJSONObject(String string) throws JSONException { | |||
public static String toString(JSONObject jo) throws JSONException { | |||
StringBuilder sb = new StringBuilder(); | |||
if (jo.has("Status-Code") && jo.has("Reason-Phrase")) { | |||
sb.append(jo.getString("HTTP-Version")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure that the refactoring update in this method is a good idea. Although it shortens the method, it hides a more interesting potential refactoring - notice how many times status-code
, reason-phrase
, method
, request-uri
, and http-version
are referenced in the conditionals. It makes the code look half-finished. Can you come up with a way to reduce the number of conditions being checked without affecting the output ordering?
@@ -1762,12 +1762,7 @@ private void populateMap(Object bean, Set<Object> objectsRecord) { | |||
Method[] methods = includeSuperClass ? klass.getMethods() : klass.getDeclaredMethods(); | |||
for (final Method method : methods) { | |||
final int modifiers = method.getModifiers(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable modifiers
is only used with the method isValidMethod
. So there is no need for this local variable here.
Instead, it should be a variable inside the method isValidMethod
and the parameter should be removed from the method signature.
Closed due to inactivity |
Hi @stleary
I have refactored some changes in following files : -