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

Properly overrides the Exception class. #159

Merged
merged 5 commits into from
Oct 18, 2015

Conversation

johnjaylward
Copy link
Contributor

Overriding the exception class should not keep it's own cause field.

@stleary
Copy link
Owner

stleary commented Oct 14, 2015

Can you say a few words about why this needs to be fixed?

@johnjaylward
Copy link
Contributor Author

The Exception base class already has a variable to store the "cause" and the super.getCause() method already handles the cause properly. This pull request simplifies the implementation as well as creates less stack overhead when throwing an exception.

@stleary
Copy link
Owner

stleary commented Oct 14, 2015

Moved to a branch commit and pull request. Please see #161

@stleary stleary closed this Oct 14, 2015
@stleary stleary reopened this Oct 14, 2015
@johnjaylward
Copy link
Contributor Author

I had already adjusted the date after seeing it modified in your PR. Not sure if that will affect the test.

@stleary
Copy link
Owner

stleary commented Oct 14, 2015

Completed unit tests successfully. No local changes needed. Will leave this pull request open for a few days for comment. This change does not change behavior or API, so will not result in a new release.

@stleary
Copy link
Owner

stleary commented Oct 17, 2015

What problem does this code solve?
The JSONException API already has a Throwable cause, initialized in the ctor and accessed via a getter.

Changes to the API, code behavior, unit tests, or documentation?
No

Will this require a new release?
No, this change will be rolled into the next release.

stleary added a commit that referenced this pull request Oct 18, 2015
Properly overrides the Exception class.
@stleary stleary merged commit b0a9507 into stleary:master Oct 18, 2015
@johnjaylward johnjaylward deleted the FixExceptionInheritance branch October 18, 2015 21:52
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.

2 participants