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

It is preferred to use Objects.requireNonNull to for null checks. #6139

Closed
edward-swirldslabs opened this issue Apr 18, 2023 · 4 comments
Closed
Assignees
Milestone

Comments

@edward-swirldslabs
Copy link
Contributor

CommonUtils.throwArgNull and ArgumentUtils.throwArgBlank both do the same activity as Objects.requireNonNull. From recent conversations the coding standard has converged on using Objects.requireNonNull as the preferred method for performing null checks to back the @NonNull attribute.

conversation 1
conversation 2
Decision/Outcome

CommonUtils.throwArgNull and ArgumentUtils.throwArgBlank should be deprecated and direct people to use Objects.requireNonNull.

@hendrikebbers
Copy link
Member

Let's double check if CommonUtils.throwArgNull and ArgumentUtils.throwArgBlank is not used anymore and is removed. In that case the issue can be closed

@mxtartaglia-sl
Copy link
Contributor

In #10433 we removed all uses of CommonUtils.throwArgNull and removed the method.
All uses of CommonUtils.throwArgBlank were changed to ArgumentUtils.throwArgBlank, and CommonUtils.throwArgBlank was removed.
There are still 12 classes in the codebase that use ArgumentUtils.throwArgBlank; changing those for Objects.requireNonNull would change the validation functionality as we would be removing the check for blank values.

I can't find in the attached discussions where this specific change is discussed.
I imagine that the functionality change is expected but could you please confirm @edward-swirldslabs.
Thanks!

@edward-swirldslabs
Copy link
Contributor Author

I'm not sure why I thought throwArgBlank could be replaced by Objects.requireNonNull. The check that a string is not all white space is not something that a check for nonNull can cover. At this point I would have to relax my request and just standardize the null check on Objects.RequireNonNull. There doesn't seem to be a standard library function that throws an exception if a string.isBlank() is true.

@mxtartaglia-sl mxtartaglia-sl added this to the v0.46 milestone Dec 20, 2023
@mxtartaglia-sl
Copy link
Contributor

Fixed with: #10433

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

No branches or pull requests

3 participants