search

stleary / JSON-java

A reference implementation of a JSON package in Java.
http://stleary.github.io/JSON-java/index.html
Other
4.54k stars 2.56k forks source link

Refactor NumberConversionUtil and toString() of CookieList & XML Classes. #831

Closed adityap27 closed 11 months ago

adityap27 commented 1 year ago

Hello @stleary,

I have made 3 refactorings.

1. Renamed Variable boolean b in toString() method of CookieList Class. boolean isEndOfPair is more meaningful as compared to just boolean b.

2. Introduced indentationSuffix variable in toString() method of XML Class. There are multiple long expressions which are using (indentFactor > 0) ? "\n" : "" sub-expression. indentationSuffix is introduced to replace this sub-expression to improve readability and reduce duplication.

3. Added method isNumericChar(...) in NumberConversionUtil Class. There are some digit-related conditions which may look complex to some developers due to multiple conditional operators. Eg: if ((initial >= '0' && initial <= '9') || initial == '-' ) and if(at1 == '0' && at2 >= '0' && at2 <= '9')

The sub-condition c >= '0' && c <= '9' is moved to a method isNumericChar(...) to reduce the complexity of these conditions and better readability.

Notes: All the unit tests are passing successfully. image

This code is compatible with Java 6.

adityap27 commented 1 year ago

Hello @stleary and @johnjaylward,

Can this be reviewed and merged please?

adityap27 commented 1 year ago

Thanks for starting the review process @stleary.

I would like to ask if there is any possibility of merging this before the 26 Nov day end? Actually, this is a part of my university course assignment, where I would be getting a few bonus points if this gets merged on time. Sorry, if this gentle request sounds casual. I really understand and respect your time.

I am happy to modify the changes as per your feedback.

stleary commented 12 months ago

@adityap27 Out of sync, please merge master to your branch.

adityap27 commented 12 months ago

@stleary - done, now in sync.

stleary commented 12 months ago

What problem does this code solve? Refactoring to improve code readability

Does the code still compile with Java6? Yes

Risks Low

Changes to the API? No

Will this require a new release? No

Should the documentation be updated? No

Does it break the unit tests? No

Was any code refactored in this commit? Yes, see first prompt

Review status APPROVED

Starting 3-day comment window