leerho commented on code in PR #475:
URL: https://github.com/apache/datasketches-java/pull/475#discussion_r1409960261


##########
src/main/java/org/apache/datasketches/kll/KllDoublesSketchSortedView.java:
##########
@@ -39,56 +41,91 @@ public final class KllDoublesSketchSortedView implements 
DoublesSortedView {
   private final double[] quantiles;
   private final long[] cumWeights; //comes in as individual weights, converted 
to cumulative natural weights
   private final long totalN;
+  private final double[] normRanks;
+  private final double maxItem;
+  private final double minItem;
 
   /**
    * Construct from elements for testing.
    * @param quantiles sorted array of quantiles
    * @param cumWeights sorted, monotonically increasing cumulative weights.
    * @param totalN the total number of items presented to the sketch.
    */
-  KllDoublesSketchSortedView(final double[] quantiles, final long[] 
cumWeights, final long totalN) {
+  KllDoublesSketchSortedView(final double[] quantiles, final long[] 
cumWeights, final long totalN,
+      final double maxItem, final double minItem) {
     this.quantiles = quantiles;
     this.cumWeights  = cumWeights;
     this.totalN = totalN;
+    this.maxItem = maxItem;
+    this.minItem = minItem;
+    final int len = cumWeights.length;
+    final double[] normRanks = new double[len];
+    for (int i = 0; i < len; i++) { normRanks[i] = (double)cumWeights[i] / 
totalN; }
+    this.normRanks = normRanks;
   }
 
   /**
    * Constructs this Sorted View given the sketch
-   * @param sk the given KllDoublesSketch.
+   * @param sketch the given KllDoublesSketch.
    */
-  public KllDoublesSketchSortedView(final KllDoublesSketch sk) {
-    this.totalN = sk.getN();
-    final double[] srcQuantiles = sk.getDoubleItemsArray();
-    final int[] srcLevels = sk.levelsArr;
-    final int srcNumLevels = sk.getNumLevels();
+  public KllDoublesSketchSortedView(final KllDoublesSketch sketch) {
+    if (sketch.isEmpty()) { throw new SketchesArgumentException(EMPTY_MSG); }
+    this.totalN = sketch.getN();
+    this.maxItem = sketch.getMaxItem();
+    this.minItem = sketch.getMinItem();
+    final double[] srcQuantiles = sketch.getDoubleItemsArray();
+    final int[] srcLevels = sketch.levelsArr;
+    final int srcNumLevels = sketch.getNumLevels();
 
-    if (!sk.isLevelZeroSorted()) {
+    if (!sketch.isLevelZeroSorted()) {

Review Comment:
   Not so simple.  The only place we care about level0 being sorted is in the 
sorted view.  This piece of code you are highlighting is actually in the method 
that creates the sorted view from the sketch.  This method happens to be in the 
sorted view class, but really should be associated with the sketch code itself 
(perhaps in its own class), which would clean up the code of the SV and make it 
more stand-alone. And it would allow us to reduce a bunch of code duplication 
across all the quantile sketches because we wouldn't need a dedicated SV for 
every q-sketch (This will be a code cleanup for later). 



-- 
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]

Reply via email to