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

Primitive Boolean expression used in FileTarget.java #555

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tanmaysharma2001
Copy link

Closes #554

@tanmaysharma2001
Copy link
Author

@yegor256 Can you review and merge this PR?

@yegor256
Copy link
Member

@tanmaysharma2001 what is the point of this modification?

@tanmaysharma2001
Copy link
Author

@yegor256 In the issue #554, as we discussed that in cases where this.overwrite is null, attempting to evaluate it in a boolean context (if (this.overwrite)) will throw a NullPointerException, which can lead to unexpected crashes or behavior.

Having a primitive Boolean Expression, properly evaluates it, in case where it is null, it will be treated in the 'else' condition making it much safer against these exceptions.

@coutvv
Copy link

coutvv commented May 9, 2024

@yegor256 In the issue #554, as we discussed that in cases where this.overwrite is null, attempting to evaluate it in a boolean context (if (this.overwrite)) will throw a NullPointerException, which can lead to unexpected crashes or behavior.

Having a primitive Boolean Expression, properly evaluates it, in case where it is null, it will be treated in the 'else' condition making it much safer against these exceptions.

@tanmaysharma2001
The main problem in the issue is not throwing NPE in the if statement, but that the field overwrite is nullable
Yegor suggest there to fix the bug by making the field into primitive type

Also using static variables is the heresy in the Elegant Object ideology

@yegor256
Copy link
Member

yegor256 commented May 9, 2024

@tanmaysharma2001 before making a fix, we must introduce a unit test, which will reproduce the error

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.

Risk of NullPointerException Due to Use of Boxed Boolean in toPath Method
3 participants