Skip to content
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

Fixed incorrect cast getting float from array #604

Merged
merged 1 commit into from
May 1, 2021

Conversation

ianlovejoy
Copy link
Contributor

Hello,

I noticed that if a double is added to a JSONArray and then retrieved with getFloat(), a class cast exception was raised. This case is handled correctly by JSONObject (as opposed to array).

The root cause turned out to be an incorrect cast. I corrected the cast and also added a test for this case. I tried to closely follow the existing coding conventions as well as the corresponding code in JSONObject.

Please let me know if you need any more information. Thanks for maintaining this project!

Sincerely,
Ian Lovejoy

Added test for getting float from array
@ianlovejoy
Copy link
Contributor Author

@johnjaylward thanks for approving these changes, it appears that there is a workflow awaiting approval as well since I am a first-time contributor, not sure if you can unstick that as well? Thanks!

@stleary
Copy link
Owner

stleary commented Apr 29, 2021

What problem does this code solve?
Exception when calling getFloat() for a valid value.

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?
No

Review status
APPROVED

@stleary
Copy link
Owner

stleary commented Apr 29, 2021

Starting 3-day comment window.

@johnjaylward
Copy link
Contributor

There is a new test @stleary

@stleary
Copy link
Owner

stleary commented Apr 29, 2021

Thanks @johnjaylward I saw the test but just didn't read it closely. Review updated.

#readthecode

@stleary stleary merged commit 143db39 into stleary:master May 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants