nfsantos commented on PR #2657:
URL: https://github.com/apache/jackrabbit-oak/pull/2657#issuecomment-3645930106

   > The only thing that could be improved, and it isn't introduced in this PR, 
is the field `AbstractPersistentCache#segmentCacheStats`. The getter is 
annotated as `@NotNull` and no null checks are present in the implementation. 
It would be more robust to make the field private and require a `@NonNull 
SegmentCacheStats` argument in the constructor. I know, that would require 
adjusting implementations as well. So maybe not the most practical idea.
   
   I see the problem, there is no mechanism to enforce that derived classes set 
the `segmentCacheStats`. Would be cleaner if there was some check, but the base 
class will fail almost immediately with a NPE if this field is null, so this 
issue would be caught in testing. I prefer to not touch this on this PR. Maybe 
another PR.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to