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


##########
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:
   Perhaps I did not make myself clear. My point was that it would be better to 
ask the sketch to do its own sorting rather than modifying it from the outside. 
This is knows as "feature envy code smell".



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