-
Notifications
You must be signed in to change notification settings - Fork 42
Check null to avoid null pointer exception #334
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
Check null to avoid null pointer exception #334
Conversation
Check if the string input is null in `getFirstLicenseToken` and `isSingleTokenString` Signed-off-by: Arthit Suriyawongkul <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're now using a Java version that support the @Nullable
annotations, we should add these to the calls.
Do we want to open a separate PR to fix the empty optional strings returning true when it should be false?
src/main/java/org/spdx/utility/compare/LicenseCompareHelper.java
Outdated
Show resolved
Hide resolved
src/main/java/org/spdx/utility/compare/LicenseCompareHelper.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Gary O'Neall <[email protected]> Signed-off-by: Arthit Suriyawongkul <[email protected]>
Co-authored-by: Gary O'Neall <[email protected]> Signed-off-by: Arthit Suriyawongkul <[email protected]>
We can fix it here as well. |
Signed-off-by: Arthit Suriyawongkul <[email protected]>
It looks like there are quite a few unit test failures. It looks like I was wrong about returning |
Thanks @goneall. I'm trying to understand the rationale behind In general, I think we like to return Another thing I'm not sure about is this line, on why we treat newline differently from other whitespaces: Spdx-Java-Library/src/main/java/org/spdx/utility/compare/LicenseCompareHelper.java Lines 215 to 217 in 3ec6b34
Do you mind to help me confirm this truth table please?
|
Signed-off-by: Arthit Suriyawongkul <[email protected]>
Signed-off-by: Arthit Suriyawongkul <[email protected]>
Signed-off-by: Arthit Suriyawongkul <[email protected]>
Signed-off-by: Arthit Suriyawongkul <[email protected]>
Signed-off-by: Arthit Suriyawongkul <[email protected]>
@bact - the table above looks correct This code was originally added to fix a corner case in the matching. I believe the actual check is to see if a text field contains more than one token. If it contains more than one token, we need to do some extra processing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thans @bact for the analysis and fix.
This seems to pass the unit tests.
We could add more documentation that the isSingleToken
is really returning 0 or 1 tokens, but that can be done in a separate PR.
I'll go ahead and merge this so we can move forward with a working solution.
Check if the string input is null in
getFirstLicenseToken
andisSingleTokenString
To fix #333