From d37cf1193c5d323f317592d0dd5241016f5e30c7 Mon Sep 17 00:00:00 2001 From: Aleksandr Sinitson <34094780+shdowtail@users.noreply.github.com> Date: Thu, 13 Jul 2023 16:52:27 +0300 Subject: [PATCH 1/2] Simple refactor, typo fixes, added logger (#639) * readability * Refactor: enhanced for loop * typo fix, added a type test, added a check to avoid NullPointerException, added logger * fix:remove extra line in docker readme (#638) --------- Co-authored-by: Amin Malek Mohammadi <76443257+aminmalek@users.noreply.github.com> --- bundles/sirix-core/logs/sirix.log | 0 .../java/org/sirix/utils/IntToObjectMap.java | 49 ++++++++++++------- 2 files changed, 32 insertions(+), 17 deletions(-) create mode 100644 bundles/sirix-core/logs/sirix.log diff --git a/bundles/sirix-core/logs/sirix.log b/bundles/sirix-core/logs/sirix.log new file mode 100644 index 000000000..e69de29bb diff --git a/bundles/sirix-core/src/main/java/org/sirix/utils/IntToObjectMap.java b/bundles/sirix-core/src/main/java/org/sirix/utils/IntToObjectMap.java index 9e7df12cc..6a67e133a 100644 --- a/bundles/sirix-core/src/main/java/org/sirix/utils/IntToObjectMap.java +++ b/bundles/sirix-core/src/main/java/org/sirix/utils/IntToObjectMap.java @@ -1,7 +1,11 @@ package org.sirix.utils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import java.util.Arrays; import java.util.Iterator; +import java.util.NoSuchElementException; /* * Licensed to the Apache Software Foundation (ASF) under one or more @@ -37,6 +41,11 @@ * @lucene.experimental */ public class IntToObjectMap implements Iterable { +/** + * Logger for this class. + */ + private static final Logger LOGGER = LoggerFactory.getLogger(IntToObjectMap.class); + /** * Implements an IntIterator which iterates over all the allocated indexes. @@ -137,9 +146,13 @@ public boolean hasNext() { @SuppressWarnings("unchecked") public T next() { + if (!iterator.hasNext()) { + throw new NoSuchElementException(); + } return (T) values[iterator.next()]; } + @Override public void remove() { iterator.remove(); } @@ -151,7 +164,7 @@ public void remove() { private static int defaultCapacity = 16; /** - * Holds the base hash entries. if the capacity is 2^N, than the base hash + * Holds the base hash entries. if the capacity is 2^N, thn the base hash * holds 2^(N+1). It can hold */ int[] baseHash; @@ -254,7 +267,7 @@ public IntToObjectMap(int capacity) { * @param e * element which is being mapped using the given key */ - private void prvt_put(int key, T e) { + private void prvtPut(int key, T e) { // Hash entry to which the new pair would be inserted int hashIndex = calcBaseHashIndex(key); @@ -300,8 +313,8 @@ public void clear() { // And setting all the next[i] to point at // i+1. - for (int i = 1; i < this.capacity; ) { - next[i] = ++i; + for (int i = 1; i < this.capacity; ++i) { + next[i] = i + 1; } // Surly, the last one should point to the 'Ground'. @@ -329,8 +342,7 @@ public boolean containsKey(int key) { * false otherwise. */ public boolean containsValue(Object o) { - for (Iterator iterator = iterator(); iterator.hasNext(); ) { - T object = iterator.next(); + for (T object : this) { if (object.equals(o)) { return true; } @@ -353,7 +365,7 @@ protected int find(int key) { // while the index does not point to the 'Ground' while (localIndex != 0) { - // returns the index found in case of of a matching key. + // returns the index found in case of a matching key. if (keys[localIndex] == key) { return localIndex; } @@ -384,7 +396,7 @@ private int findForRemove(int key, int baseHashIndex) { // while the index does not point to the 'Ground' while (index != 0) { - // returns the index found in case of of a matching key. + // returns the index found in case of a matching key. if (keys[index] == key) { return index; } @@ -418,14 +430,14 @@ public T get(int key) { */ @SuppressWarnings("unchecked") protected void grow() { - IntToObjectMap that = new IntToObjectMap(this.capacity * 2); + IntToObjectMap that = new IntToObjectMap<>(this.capacity * 2); // Iterates fast over the collection. Any valid pair is put into the new // map without checking for duplicates or if there's enough space for // it. for (IndexIterator iterator = new IndexIterator(); iterator.hasNext(); ) { int index = iterator.next(); - that.prvt_put(this.keys[index], (T) this.values[index]); + that.prvtPut(this.keys[index], (T) this.values[index]); } // Copy that's data into this. @@ -465,7 +477,7 @@ public IntIterator keyIterator() { @SuppressWarnings("unused") private void printBaseHash() { for (int i = 0; i < this.baseHash.length; i++) { - System.out.println(i + ".\t" + baseHash[i]); + LOGGER.info("{}.\t{}", i, baseHash[i]); } } @@ -491,13 +503,13 @@ public T put(int key, T e) { // Is there enough room for a new pair? if (size == capacity) { - // No? Than grow up! + // No? Then grow up! grow(); } // Now that everything is set, the pair can be just put inside with no // worries. - prvt_put(key, e); + prvtPut(key, e); return null; } @@ -547,8 +559,8 @@ public Object[] toArray() { Object[] array = new Object[size]; // Iterates over the values, adding them to the array. - for (Iterator iterator = iterator(); iterator.hasNext(); ) { - array[++j] = iterator.next(); + for (T t : this) { + array[++j] = t; } return array; } @@ -580,7 +592,7 @@ public T[] toArray(T[] a) { @Override public String toString() { - StringBuffer sb = new StringBuffer(); + StringBuilder sb = new StringBuilder(); sb.append('{'); IntIterator keyIterator = keyIterator(); while (keyIterator.hasNext()) { @@ -605,6 +617,9 @@ public int hashCode() { @SuppressWarnings("unchecked") @Override public boolean equals(Object o) { + if (!(o instanceof IntToObjectMap)) { + return false; + } IntToObjectMap that = (IntToObjectMap) o; if (that.size() != this.size()) { return false; @@ -619,7 +634,7 @@ public boolean equals(Object o) { T v1 = this.get(key); T v2 = that.get(key); - if ((v1 == null && v2 != null) || (v1 != null && v2 == null) || (!v1.equals(v2))) { + if ((v1 == null && v2 != null) || (v1 != null && v2 == null) || (v1!= null && !v1.equals(v2))) { return false; } } From 1e51ae53a88a084b440c5b5252137b73dca9ae4c Mon Sep 17 00:00:00 2001 From: Aleksandr Sinitson <34094780+shdowtail@users.noreply.github.com> Date: Fri, 14 Jul 2023 16:19:33 +0300 Subject: [PATCH 2/2] Added Null checks + minor fixes (#640) * readability * Refactor: enhanced for loop * typo fix, added a type test, added a check to avoid NullPointerException, added logger * fix:remove extra line in docker readme (#638) * typo in method * minor fix * reordered modifiers * final method in final class * Condition 'v2 != null' is always 'true' when reached * final method in final class * identical method * final method in final class * null check * null checks and redundant conditional * null check * added a private constructor, error messages --------- Co-authored-by: Amin Malek Mohammadi <76443257+aminmalek@users.noreply.github.com> --- ...tted_changes_before_rebase_[Changes].patch | 100 ++++++++++++++++++ .../src/main/java/org/sirix/utils/Calc.java | 83 +++++++++------ 2 files changed, 149 insertions(+), 34 deletions(-) create mode 100644 Uncommitted_changes_before_rebase_[Changes].patch diff --git a/Uncommitted_changes_before_rebase_[Changes].patch b/Uncommitted_changes_before_rebase_[Changes].patch new file mode 100644 index 000000000..e90f20696 --- /dev/null +++ b/Uncommitted_changes_before_rebase_[Changes].patch @@ -0,0 +1,100 @@ +Subject: [PATCH] Uncommitted changes before rebase [Changes] +--- +Index: bundles/sirix-core/src/main/java/org/sirix/utils/IntToObjectMap.java +IDEA additional info: +Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP +<+>UTF-8 +=================================================================== +diff --git a/bundles/sirix-core/src/main/java/org/sirix/utils/IntToObjectMap.java b/bundles/sirix-core/src/main/java/org/sirix/utils/IntToObjectMap.java +--- a/bundles/sirix-core/src/main/java/org/sirix/utils/IntToObjectMap.java ++++ b/bundles/sirix-core/src/main/java/org/sirix/utils/IntToObjectMap.java +@@ -1,7 +1,11 @@ + package org.sirix.utils; + ++import org.slf4j.Logger; ++import org.slf4j.LoggerFactory; ++ + import java.util.Arrays; + import java.util.Iterator; ++import java.util.NoSuchElementException; + + /* + * Licensed to the Apache Software Foundation (ASF) under one or more +@@ -137,9 +141,13 @@ + + @SuppressWarnings("unchecked") + public T next() { ++ if (!iterator.hasNext()) { ++ throw new NoSuchElementException(); ++ } + return (T) values[iterator.next()]; + } + ++ @Override + public void remove() { + iterator.remove(); + } +@@ -151,7 +159,7 @@ + private static int defaultCapacity = 16; + + /** +- * Holds the base hash entries. if the capacity is 2^N, than the base hash ++ * Holds the base hash entries. if the capacity is 2^N, thn the base hash + * holds 2^(N+1). It can hold + */ + int[] baseHash; +@@ -301,7 +309,7 @@ + // And setting all the next[i] to point at + // i+1. + for (int i = 1; i < this.capacity; ) { +- next[i] = ++i; ++ next[i] = i + 1; + } + + // Surly, the last one should point to the 'Ground'. +@@ -329,8 +337,7 @@ + * false otherwise. + */ + public boolean containsValue(Object o) { +- for (Iterator iterator = iterator(); iterator.hasNext(); ) { +- T object = iterator.next(); ++ for (T object : this) { + if (object.equals(o)) { + return true; + } +@@ -353,7 +360,7 @@ + + // while the index does not point to the 'Ground' + while (localIndex != 0) { +- // returns the index found in case of of a matching key. ++ // returns the index found in case of a matching key. + if (keys[localIndex] == key) { + return localIndex; + } +@@ -384,7 +391,7 @@ + + // while the index does not point to the 'Ground' + while (index != 0) { +- // returns the index found in case of of a matching key. ++ // returns the index found in case of a matching key. + if (keys[index] == key) { + return index; + } +@@ -465,7 +472,7 @@ + @SuppressWarnings("unused") + private void printBaseHash() { + for (int i = 0; i < this.baseHash.length; i++) { +- System.out.println(i + ".\t" + baseHash[i]); ++ LOGGER.info("{}.\t{}", i, baseHash[i]); + } + } + +@@ -491,7 +498,7 @@ + + // Is there enough room for a new pair? + if (size == capacity) { +- // No? Than grow up! ++ // No? Then grow up! + grow(); + } + diff --git a/bundles/sirix-core/src/main/java/org/sirix/utils/Calc.java b/bundles/sirix-core/src/main/java/org/sirix/utils/Calc.java index e395797c5..5f55e4f67 100644 --- a/bundles/sirix-core/src/main/java/org/sirix/utils/Calc.java +++ b/bundles/sirix-core/src/main/java/org/sirix/utils/Calc.java @@ -4,6 +4,7 @@ import java.math.BigDecimal; import java.math.BigInteger; import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.util.Arrays; /** @@ -13,9 +14,11 @@ * */ public final class Calc { + private Calc() {} - private static final Charset UTF8 = Charset.forName("UTF-8"); - private static final int NONENULL = Integer.MAX_VALUE; + private static final String ERROR_MSG = "Input byte array cannot be null"; + private static final Charset UTF8 = StandardCharsets.UTF_8; + private static final int MAX_INT = Integer.MAX_VALUE; public static byte[] fromBigDecimal(BigDecimal i) { BigInteger bi = i.unscaledValue(); byte[] tmp = bi.toByteArray(); @@ -38,6 +41,9 @@ public static byte[] fromBigDecimal(BigDecimal i) { } public static BigDecimal toBigDecimal(byte[] b) { + if (b == null) { + throw new IllegalArgumentException(ERROR_MSG); + } if ((b[0] & 0x80) == 0) { // bit 0 not set: 6 bits are sufficient // to encode scale unsigned @@ -103,6 +109,9 @@ public static void fromString(String s, byte[] b, int off) { } public static int toUIntVar(byte[] b) { + if (b == null) { + throw new IllegalArgumentException(ERROR_MSG); + } int len = b.length; if (len == 1) { return b[0] & 0xFF; @@ -152,6 +161,9 @@ public static int toInt(byte[] b) { } public static int toInt(byte[] b, int off) { + if (b == null) { + throw new IllegalArgumentException(ERROR_MSG); + } return ((b[off++] & 0xFF) << 24) | ((b[off++] & 0xFF) << 16) | ((b[off++] & 0xFF) << 8) | (b[off++] & 0xFF); } @@ -175,6 +187,9 @@ public static long toLong(byte[] b) { } public static long toLong(byte[] b, int off) { + if (b == null) { + throw new IllegalArgumentException(ERROR_MSG); + } return ((((long) b[off++] & 0xFF) << 56) | (((long) b[off++] & 0xFF) << 48) | (((long) b[off++] & 0xFF) << 40) | (((long) b[off++] & 0xFF) << 32) | (((long) b[off++] & 0xFF) << 24) | (((long) b[off++] & 0xFF) << 16) @@ -232,8 +247,8 @@ public static void fromDouble(float i, byte[] b, int off) { public static int compare(byte[] v1, byte[] v2) { // a null value is interpreted as EOF (= highest possible value) - if(comparsionOfNull(v1, v2) != NONENULL) - return comparsionOfNull(v1, v2); + if(comparisonOfNull(v1, v2) != MAX_INT) + return comparisonOfNull(v1, v2); int len1 = v1.length; int len2 = v2.length; @@ -252,8 +267,8 @@ public static int compare(byte[] v1, byte[] v2) { public static int compare(byte[] v1, int off1, int len1, byte[] v2, int off2, int len2) { // a null value is interpreted as EOF (= highest possible value) - if(comparsionOfNull(v1, v2) != NONENULL) - return comparsionOfNull(v1, v2); + if(comparisonOfNull(v1, v2) != MAX_INT) + return comparisonOfNull(v1, v2); int len = Math.min(len1, len2); int pos = -1; while (++pos < len) { @@ -274,21 +289,21 @@ public static int compare(byte[] v1, int off1, int len1, byte[] v2, int off2, in * @param v2 the second value to be tested. * @return -1, if v2 is null but not v1, 1 for vice versa, 0 for both being null, and int max for when neither is null. */ - private final static int comparsionOfNull(byte[] v1, byte[] v2){ + private static int comparisonOfNull(byte[] v1, byte[] v2){ if(v1 == null && v2 == null) return 0; else if(v1 != null && v2 == null) return -1; - else if(v1 == null && v2 != null) + else if(v1 == null) return 1; else - return NONENULL; + return MAX_INT; } - public final static int compareAsPrefix(byte[] v1, byte[] v2) { + public static int compareAsPrefix(byte[] v1, byte[] v2) { // a null value is interpreted as EOF (= highest possible value) - if(comparsionOfNull(v1, v2) != NONENULL) - return comparsionOfNull(v1, v2); + if(comparisonOfNull(v1, v2) != MAX_INT) + return comparisonOfNull(v1, v2); int len1 = v1.length; int len2 = v2.length; @@ -309,8 +324,8 @@ public static int compareU(byte[] v1, byte[] v2) { public static int compareU(byte[] v1, int off1, int len1, byte[] v2, int off2, int len2) { // a null value is interpreted as EOF (= highest possible value) - if(comparsionOfNull(v1, v2) != NONENULL) - return comparsionOfNull(v1, v2); + if(comparisonOfNull(v1, v2) != MAX_INT) + return comparisonOfNull(v1, v2); int len = Math.min(len1, len2); int pos = -1; while (++pos < len) { @@ -323,10 +338,10 @@ public static int compareU(byte[] v1, int off1, int len1, byte[] v2, int off2, i return len1 - len2; } - public final static int compareUAsPrefix(byte[] v1, byte[] v2) { + public static int compareUAsPrefix(byte[] v1, byte[] v2) { // a null value is interpreted as EOF (= highest possible value) - if(comparsionOfNull(v1, v2) != Integer.MAX_VALUE) - return comparsionOfNull(v1, v2); + if(comparisonOfNull(v1, v2) != Integer.MAX_VALUE) + return comparisonOfNull(v1, v2); int len1 = v1.length; int len2 = v2.length; int len = Math.min(len1, len2); @@ -343,11 +358,11 @@ public final static int compareUAsPrefix(byte[] v1, byte[] v2) { public static int compareUIntVar(byte[] v1, byte[] v2) { // a null value is interpreted as EOF (= highest possible value) - if(comparsionOfNull(v1, v2) != NONENULL) - return comparsionOfNull(v1, v2); + if(comparisonOfNull(v1, v2) != MAX_INT) + return comparisonOfNull(v1, v2); int i1 = toUIntVar(v1); int i2 = toUIntVar(v2); - return (i1 < i2) ? -1 : (i1 == i2) ? 0 : 1; + return Integer.compare(i1, i2); } public static int compareInt(byte[] v1, byte[] v2) { @@ -357,11 +372,11 @@ public static int compareInt(byte[] v1, byte[] v2) { public static int compareInt(byte[] v1, int off1, byte[] v2, int off2) { // a null value is interpreted as EOF (= highest possible value) - if(comparsionOfNull(v1, v2) != NONENULL) - return comparsionOfNull(v1, v2); + if(comparisonOfNull(v1, v2) != MAX_INT) + return comparisonOfNull(v1, v2); int i1 = toInt(v1, off1); int i2 = toInt(v2, off2); - return (i1 < i2) ? -1 : (i1 == i2) ? 0 : 1; + return Integer.compare(i1, i2); } public static int compareLong(byte[] v1, byte[] v2) { @@ -371,17 +386,17 @@ public static int compareLong(byte[] v1, byte[] v2) { public static int compareLong(byte[] v1, int off1, byte[] v2, int off2) { // a null value is interpreted as EOF (= highest possible value) - if(comparsionOfNull(v1, v2) != NONENULL) - return comparsionOfNull(v1, v2); + if(comparisonOfNull(v1, v2) != MAX_INT) + return comparisonOfNull(v1, v2); long i1 = toLong(v1, off1); long i2 = toLong(v2, off2); - return (i1 < i2) ? -1 : (i1 == i2) ? 0 : 1; + return Long.compare(i1, i2); } public static int compareDouble(byte[] v1, byte[] v2) { // a null value is interpreted as EOF (= highest possible value) - if(comparsionOfNull(v1, v2) != NONENULL) - return comparsionOfNull(v1, v2); + if(comparisonOfNull(v1, v2) != MAX_INT) + return comparisonOfNull(v1, v2); double d1 = toDouble(v1); double d2 = toDouble(v2); return Double.compare(d1, d2); @@ -389,8 +404,8 @@ public static int compareDouble(byte[] v1, byte[] v2) { public static int compareFloat(byte[] v1, byte[] v2) { // a null value is interpreted as EOF (= highest possible value) - if(comparsionOfNull(v1, v2) != NONENULL) - return comparsionOfNull(v1, v2); + if(comparisonOfNull(v1, v2) != MAX_INT) + return comparisonOfNull(v1, v2); float f1 = toFloat(v1); float f2 = toFloat(v2); return Float.compare(f1, f2); @@ -398,8 +413,8 @@ public static int compareFloat(byte[] v1, byte[] v2) { public static int compareBigInteger(byte[] v1, byte[] v2) { // a null value is interpreted as EOF (= highest possible value) - if(comparsionOfNull(v1, v2) != NONENULL) - return comparsionOfNull(v1, v2); + if(comparisonOfNull(v1, v2) != MAX_INT) + return comparisonOfNull(v1, v2); BigInteger i1 = toBigInteger(v1); BigInteger i2 = toBigInteger(v2); return i1.compareTo(i2); @@ -407,8 +422,8 @@ public static int compareBigInteger(byte[] v1, byte[] v2) { public static int compareBigDecimal(byte[] v1, byte[] v2) { // a null value is interpreted as EOF (= highest possible value) - if (comparsionOfNull(v1, v2) != NONENULL) - return comparsionOfNull(v1, v2); + if (comparisonOfNull(v1, v2) != MAX_INT) + return comparisonOfNull(v1, v2); BigDecimal i1 = toBigDecimal(v1); BigDecimal i2 = toBigDecimal(v2); return i1.compareTo(i2);