-
Notifications
You must be signed in to change notification settings - Fork 47
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
[Proposal] Use unboxed boolean types in ResourceHandlerRequest #341
Comments
We use boxed type because the property could not exist from the request, therefore, it could be null in such case. It makes sense that if we know the default value of those boolean properties, then it's better to provide an extra helper methods on |
@wbingli if my understanding is correct, just providing helper methods that return primitive types to support backward compatibility instead of changing the data type of the actual instance variable is suggested. But what would the default values of these snapshotRequested, rollback and driftable be? false? |
There are 2 recent changes that landed in
ResourceHandlerRequest
: ebca1dc#diff-adfb207dac8dff5107b68b36280ccad4525f92176184ad2ffff9aa7e591b2e8a and e34f047#diff-adfb207dac8dff5107b68b36280ccad4525f92176184ad2ffff9aa7e591b2e8a.Both changes favor boxed Boolean over primitive types.
As a result, the usage of this class implies on importing BooleanUtils or a similar helper in order to avoid constructions like:
if (request.getSnapshotRequested() != null && request.getSnapshotRequested() == true) { ... }
.And the code that is performing a "naive" boolean comparison like:
if (request.getSnapshotRequested()) { ... }
will fail with a null-pointer exception.The same stands for the rollback flag.
I wonder if var type boxing was done for some specific reasons? If there are no strict reasons, I would vote for unboxing them to simplify condition checking.
The text was updated successfully, but these errors were encountered: