mumrah commented on code in PR #18684:
URL: https://github.com/apache/kafka/pull/18684#discussion_r1977651214
##########
server-common/src/main/java/org/apache/kafka/timeline/Snapshot.java:
##########
@@ -47,7 +47,9 @@ <T extends Delta> T getDelta(Revertable owner) {
}
void setDelta(Revertable owner, Delta delta) {
- map.put(owner, delta);
+ if (map != null) {
+ map.put(owner, delta);
+ }
Review Comment:
Sorry, my comment "sanity checks" was probably a bit too vague.
> However, since we can't guarantee that the caller properly disposes of the
object and does not make any further calls which touch map, it would be good to
include some sanity checks.
We should not silently ignore a null `map`. That would be a violation of the
implied contract here which is that a Snapshot cannot be used again after
`erase` is called.
What we can do is add an explicit not-null assertion with
`Objects.requireNonNull`. This will still throw an NPE, but it makes the code
more explicit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
