leerho commented on code in PR #682:
URL: https://github.com/apache/datasketches-java/pull/682#discussion_r2255445842
##########
src/main/java/org/apache/datasketches/quantiles/DirectUpdateDoublesSketch.java:
##########
@@ -99,6 +100,11 @@ static DirectUpdateDoublesSketch newInstance(final int k,
final MemorySegment ds
return new DirectUpdateDoublesSketch(k, dstSeg, mSegReq);
}
+ static HeapUpdateDoublesSketch heapify(final DirectUpdateDoublesSketch skIn)
{
+ final MemorySegment segIn = skIn.getMemorySegment();
Review Comment:
This method is package-private. Looking at the call hierarchy, this method
is _only_ called from a method in the same class:
```
UpdateDoublesSketch getSketchAndReset() {
final HeapUpdateDoublesSketch skCopy = heapify(this);
...
```
Where the parameter passed is the owning class, which has to exist. So it
can never be null.
Nonetheless, your observation is fair, and applies generally to the whole
library. My preferred practice is to check parameter validity on the public
methods where the parameters originate, or at least higher up the call stack,
if possible, rather than on the lower level internal methods. But this code
has multiple authors, which make it difficult to enforce.
I think it is OK to leave this particular method the way it is. But come
back later with a strategy, either through testing or more careful design
across the entire library to avoid difficult to debug null pointers.
Thank you for your observation!
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
