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

Fix NPE when branch is orphaned #48

Merged
merged 2 commits into from
Nov 19, 2019
Merged

Fix NPE when branch is orphaned #48

merged 2 commits into from
Nov 19, 2019

Conversation

bjrke
Copy link
Contributor

@bjrke bjrke commented Nov 19, 2019

This closes #47

return new PullRequestInfo(jsonObject.get("key").getAsString(), jsonObject.get("branch").getAsString(),
jsonObject.get("base").getAsString(), parsedDate);
base, parsedDate);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could probably have been more succinctly written as Optional.ofNullable(jsonObject.get("base")).orElse(null) which would prevent the need for variable assignment and additional null checks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the getAsString transformation in your code example is missing, which makes it much longer and unreadable if not in an extra var

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got no issue with it being in an extra var, it was the null checks that seemed unnecessary. Your change looks good to me. I presume you've verified this now works for you locally?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it works in production on our server

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and the nullcheck is necessary to prevent the NPEs. Otherwise you cannot call getAsString. Optional.orElse does nothing differently

@mc1arke mc1arke merged commit 819638c into mc1arke:master Nov 19, 2019
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.

NullPointerException in JSON Deserializer
2 participants