ZOOKEEPER-3988: Protect zkServer from NullPointerException #1520
Conversation
eolivelli
left a comment
eolivelli
left a comment
There was a problem hiding this comment.
I am not sure that this code is better than before.
Which are the benefits ?
also I see that we are using a little more CPU cycles (more method calls)
| } | ||
| if (zkServer.shouldThrottle(outstandingCount.incrementAndGet())) { | ||
| disableRecv(false); | ||
| if (zkServer.isPresent() |
There was a problem hiding this comment.
I am not sure this is what we want,
previously we would have seen an NPE here,
now we aren't doing anything.
This is just an example,
can you please explain better the intent of the patch ?
There was a problem hiding this comment.
My intent was to remove the NPE. It is valid that zkServer could be null and yet there is no check for it (please review the original JIRA).
Is there something else that should be done here? Comments make it clear that zkServer has a valid value of null.
|
@eolivelli For a primer on https://github.com/google/guava/wiki/UsingAndAvoidingNullExplained#optional |
|
@belugabehr thanks for the pointer. I know about Optional benefits. Let me think more about it |
2 participants
No description provided.