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


##########
src/main/java/org/apache/datasketches/kll/KllMemoryValidate.java:
##########
@@ -45,157 +32,162 @@
 import static org.apache.datasketches.kll.KllPreambleUtil.getMemoryNumLevels;
 import static org.apache.datasketches.kll.KllPreambleUtil.getMemoryPreInts;
 import static org.apache.datasketches.kll.KllPreambleUtil.getMemorySerVer;
-import static 
org.apache.datasketches.kll.KllPreambleUtil.getMemorySingleItemFlag;
 import static org.apache.datasketches.kll.KllSketch.SketchType.DOUBLES_SKETCH;
+import static org.apache.datasketches.kll.KllSketch.SketchType.FLOATS_SKETCH;
+import static org.apache.datasketches.kll.KllSketch.SketchType.ITEMS_SKETCH;
 
+import org.apache.datasketches.common.ArrayOfItemsSerDe;
 import org.apache.datasketches.common.Family;
 import org.apache.datasketches.common.SketchesArgumentException;
+import org.apache.datasketches.kll.KllSketch.SketchStructure;
 import org.apache.datasketches.kll.KllSketch.SketchType;
 import org.apache.datasketches.memory.Memory;
-import org.apache.datasketches.memory.WritableMemory;
 
 /**
  * This class performs all the error checking of an incoming Memory object and 
extracts the key fields in the process.
  * This is used by all KLL sketches that read or import Memory objects.
- *
  * @author lrhodes
  *
  */
 final class KllMemoryValidate {
+  final Memory srcMem;
+  final ArrayOfItemsSerDe<?> serDe;
+  final SketchType sketchType;
+  final SketchStructure sketchStructure;
+
   // first 8 bytes of preamble
-  final int preInts;
-  final int serVer;
-  final int familyID;
-  final int flags;
-  final int k;
-  final int m;
-  //last byte is unused
-  
+  final int preInts;  //used by KllPreambleUtil
+  final int serVer;   //used by KllPreambleUtil
+  final int familyID; //used by KllPreambleUtil
+  final int flags;    //used by KllPreambleUtil
+  final int k;        //used multiple places
+  final int m;        //used multiple places
+  //byte 7 is unused
+
   //Flag bits:
-  final boolean empty;
-  final boolean level0Sorted;
-  final boolean singleItem;
-  //From SerVer
-  private boolean serialVersionUpdatable;
-  
+  final boolean emptyFlag;        //used multiple places
+  final boolean level0SortedFlag; //used multiple places
+
   // depending on the layout, the next 8-16 bytes of the preamble, may be 
derived by assumption.
-  // For example, if the layout is compact & empty, n = 0, if compact and 
single, n = 1. 
-  long n;        //8 bytes (if present)
-  int minK;      //2 bytes (if present)
-  int numLevels; //1 byte  (if present)
-  //unused byte
-  int[] levelsArr; //starts at byte 20, adjusted to include top index here
-  
-  // derived, other
-  int sketchBytes;
-  private int typeBytes;
-  
+  // For example, if the layout is compact & empty, n = 0, if compact and 
single, n = 1.
+  long n;        //8 bytes (if present), used multiple places
+  int minK;      //2 bytes (if present), used multiple places
+  int numLevels; //1 byte  (if present), used by KllPreambleUtil
+  //skip unused byte
+  int[] levelsArr; //starts at byte 20, adjusted to include top index here, 
used multiple places
+
+  // derived.
+  int sketchBytes = 0; //used by KllPreambleUtil
+  private int typeBytes = 0; //always 0 for generic
+
   KllMemoryValidate(final Memory srcMem, final SketchType sketchType) {
+    this(srcMem, sketchType, null);
+  }
+
+  KllMemoryValidate(final Memory srcMem, final SketchType sketchType, final 
ArrayOfItemsSerDe<?> serDe) {
+    final long memCapBytes = srcMem.getCapacity();
+    if (memCapBytes < 8) { throw new 
SketchesArgumentException(MEMORY_TOO_SMALL + memCapBytes); }
+    this.srcMem = srcMem;
+    this.sketchType = sketchType;
+    this.serDe = serDe;
     preInts = getMemoryPreInts(srcMem);
     serVer = getMemorySerVer(srcMem);
+    sketchStructure = SketchStructure.getSketchStructure(preInts, serVer);
     familyID = getMemoryFamilyID(srcMem);
-    if (familyID != Family.KLL.getID()) { memoryValidateThrow(SRC_NOT_KLL, 
familyID); }
+    if (familyID != Family.KLL.getID()) { throw new 
SketchesArgumentException(SRC_NOT_KLL + familyID); }
     flags = getMemoryFlags(srcMem);
     k = getMemoryK(srcMem);
     m = getMemoryM(srcMem);
     KllHelper.checkM(m);
     KllHelper.checkK(k, m);
-    
-    empty = getMemoryEmptyFlag(srcMem);
-    level0Sorted  = getMemoryLevelZeroSortedFlag(srcMem);
-    singleItem = getMemorySingleItemFlag(srcMem);
-    
-    serialVersionUpdatable = serVer == SERIAL_VERSION_UPDATABLE;
-    typeBytes = (sketchType == DOUBLES_SKETCH) ? Double.BYTES : Float.BYTES;
-
-    if (serialVersionUpdatable) { updatableMemFormatValidate((WritableMemory) 
srcMem); }
-    else { compactMemoryValidate(srcMem); }
+    //flags
+    emptyFlag = getMemoryEmptyFlag(srcMem);
+    level0SortedFlag  = getMemoryLevelZeroSortedFlag(srcMem);
+    if (sketchType == DOUBLES_SKETCH) { typeBytes = Double.BYTES; }
+    else if (sketchType == FLOATS_SKETCH) { typeBytes = Float.BYTES; }
+    else { typeBytes = 0; }
+    validate();
   }
 
-  private void compactMemoryValidate(final Memory srcMem) { //FOR HEAPIFY.  
NOT UPDATABLE
-    if (empty && singleItem) { memoryValidateThrow(EMPTYBIT_AND_SINGLEBIT, 
flags); }
-    final int sw = (empty ? 1 : 0) | (singleItem ? 4 : 0);
+  private void validate() {
 
-    switch (sw) {
-      case 0: { //FULL_COMPACT
-        if (preInts != PREAMBLE_INTS_FULL) { 
memoryValidateThrow(INVALID_PREINTS, preInts); }
-        if (serVer != SERIAL_VERSION_EMPTY_FULL) { 
memoryValidateThrow(EMPTYBIT_AND_SER_VER, serVer); }
+    switch (sketchStructure) {
+      case COMPACT_FULL: {
+        if (emptyFlag) { throw new 
SketchesArgumentException(EMPTY_FLAG_AND_COMPACT_FULL); }
         n = getMemoryN(srcMem);
+        //if (n <= 1) { memoryValidateThrow(N_AND_COMPACT_FULL); }

Review Comment:
   do we need to keep this commented-out line?



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