Skip to content

Commit

Permalink
Merge pull request #472 from apache/fix_spot_bugs_issues2
Browse files Browse the repository at this point in the history
This fixes the remaining spotBugs issues.
  • Loading branch information
leerho authored Aug 31, 2023
2 parents b5ed0b5 + 8484dad commit 2dfa934
Show file tree
Hide file tree
Showing 18 changed files with 161 additions and 51 deletions.
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -150,3 +150,9 @@ In Eclipse, open the project *Properties / Java Build Path / Module Dependencies

**NOTE:** If you execute *Maven/Update Project...* from Eclipse with the option *Update project configuration from pom.xml* checked, all of the above will be erased, and you will have to redo it.

## Known Issues

#### SpotBugs

* Make sure you configure SpotBugs with the /tools/FindBugsExcludeFilter.xml file. Otherwise, you will get a lot of false positive or low risk issues that we have examined and exliminated with this exclusion file.

2 changes: 1 addition & 1 deletion src/main/java/org/apache/datasketches/common/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,7 @@ public static int numDigits(int n) {
* @return the given number to a string prepended with spaces
*/
public static String intToFixedLengthString(final int number, final int length) {
final String num = Integer.valueOf(number).toString();
final String num = Integer.toString(number);
return characterPad(num, length, ' ', false);
}

Expand Down
13 changes: 8 additions & 5 deletions src/main/java/org/apache/datasketches/kll/KllItemsSketch.java
Original file line number Diff line number Diff line change
Expand Up @@ -240,10 +240,10 @@ public double[] getRanks(final T[] quantiles, final QuantileSearchCriteria searc
}

@Override
public KllItemsSketchSortedView<T> getSortedView() {
public final KllItemsSketchSortedView<T> getSortedView() {
if (isEmpty()) { throw new SketchesArgumentException(EMPTY_MSG); }
refreshSortedView();
return kllItemsSV;
return refreshSortedView();
//return kllItemsSV; //SpotBugs EI_EXPOSE_REP, Suppressed by FindBugsExcludeFilter
}

@Override
Expand Down Expand Up @@ -301,8 +301,11 @@ MemoryRequestServer getMemoryRequestServer() {
@Override
abstract int getMinMaxSizeBytes();

private final void refreshSortedView() {
kllItemsSV = (kllItemsSV == null) ? new KllItemsSketchSortedView<T>(this) : kllItemsSV;
private final KllItemsSketchSortedView<T> refreshSortedView() {
final KllItemsSketchSortedView<T> sv = (kllItemsSV == null)
? kllItemsSV = new KllItemsSketchSortedView<T>(this)
: kllItemsSV;
return sv;
}

abstract T[] getRetainedItemsArray();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,8 @@ public class KllItemsSketchSortedView<T> implements GenericSortedView<T> {
* @param totalN the total number of items presented to the sketch.
* @param minItem used to extract the type of T
* @param comparator the Comparator for type T
*
*/
public KllItemsSketchSortedView(
KllItemsSketchSortedView(
final T[] quantiles,
final long[] cumWeights,
final long totalN,
Expand All @@ -77,7 +76,7 @@ public KllItemsSketchSortedView(
* Constructs this Sorted View given the sketch
* @param sk the given KllItemsSketch.
*/
public KllItemsSketchSortedView(final KllItemsSketch<T> sk) {
KllItemsSketchSortedView(final KllItemsSketch<T> sk) {
this.totalN = sk.getN();
this.minItem = sk.getMinItem();
final Object[] srcQuantiles = sk.getTotalItemsArray();
Expand Down
10 changes: 7 additions & 3 deletions src/main/java/org/apache/datasketches/quantiles/ItemsSketch.java
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,8 @@ public void putMemory(final WritableMemory dstMem, final ArrayOfItemsSerDe<T> se

@Override
public GenericSortedView<T> getSortedView() {
return new ItemsSketchSortedView<T>(this);
if (isEmpty()) { throw new SketchesArgumentException(EMPTY_MSG); }
return refreshSortedView();
}

@Override
Expand All @@ -605,8 +606,11 @@ public void update(final T item) {

// Restricted

private final void refreshSortedView() {
classicQisSV = (classicQisSV == null) ? new ItemsSketchSortedView<T>(this) : classicQisSV;
private final ItemsSketchSortedView<T> refreshSortedView() {
final ItemsSketchSortedView<T> sv = (classicQisSV == null)
? classicQisSV = new ItemsSketchSortedView<T>(this)
: classicQisSV;
return sv;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
* @author Kevin Lang
* @author Alexander Saydakov
*/
public final class ItemsSketchSortedView<T> implements GenericSortedView<T> {
public class ItemsSketchSortedView<T> implements GenericSortedView<T> {
private final T[] quantiles;
private final long[] cumWeights; //comes in as individual weights, converted to cumulative natural weights
private final long totalN;
Expand All @@ -54,7 +54,10 @@ public final class ItemsSketchSortedView<T> implements GenericSortedView<T> {
* @param totalN the total number of items presented to the sketch.
* @param comparator comparator for type T
*/
ItemsSketchSortedView(final T[] quantiles, final long[] cumWeights, final long totalN,
ItemsSketchSortedView(
final T[] quantiles,
final long[] cumWeights,
final long totalN,
final Comparator<T> comparator) {
this.quantiles = quantiles;
this.cumWeights = cumWeights;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ public class GenericSortedViewIterator<T> implements SortedViewIterator {
private int index;

public GenericSortedViewIterator(final T[] quantiles, final long[] cumWeights) {
this.quantiles = quantiles;
this.cumWeights = cumWeights;
this.quantiles = quantiles; //SpotBugs EI_EXPOSE_REP2 suppressed by FindBugsExcludeFilter
this.cumWeights = cumWeights; //SpotBugs EI_EXPOSE_REP2 suppressed by FindBugsExcludeFilter
this.totalN = (cumWeights.length > 0) ? cumWeights[cumWeights.length - 1] : 0;
index = -1;
}
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/org/apache/datasketches/common/TestUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
*/
public final class TestUtil {

private static String userDir = System.getProperty("user.dir");
private static final String userDir = System.getProperty("user.dir");

/**
* TestNG group constants
Expand All @@ -41,17 +41,17 @@ public final class TestUtil {
/**
* The full target Path for Java serialized sketches to be tested by other languages.
*/
public static Path javaPath = createPath("target/java_generated_files");
public static final Path javaPath = createPath("target/java_generated_files");

/**
* The full target Path for C++ serialized sketches to be tested by Java.
*/
public static Path cppPath = createPath("target/cpp_generated_files");
public static final Path cppPath = createPath("target/cpp_generated_files");

/**
* The full target Path for historical C++ serialized sketches to be tested by Java.
*/
public static Path cppHistPath = createPath("src/test/resources");
public static final Path cppHistPath = createPath("src/test/resources");

private static Path createPath(final String projectLocalDir) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ public int compare(final String s1, final String s2) {
try {
final int i1 = Integer.parseInt(s1);
final int i2 = Integer.parseInt(s2);
return Integer.valueOf(i1).compareTo(i2);
return Integer.compare(i1, i2);
} catch (NumberFormatException e) {
throw new RuntimeException(e);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.apache.datasketches.kll;

import java.util.Comparator;

/**
* For testing only
*/
public class KllItemsSketchSortedViewString extends KllItemsSketchSortedView<String> {

public KllItemsSketchSortedViewString(
final String[] quantiles,
final long[] cumWeights,
final long totalN,
final String minItem,
final Comparator<String> comparator) {
super(quantiles, cumWeights, totalN, minItem, comparator);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public class KllItemsSketchTest {
private static final double PMF_EPS_FOR_K_128 = 0.025; // PMF rank error (epsilon) for k=128
private static final double PMF_EPS_FOR_K_256 = 0.013; // PMF rank error (epsilon) for k=256
private static final double NUMERIC_NOISE_TOLERANCE = 1E-6;
public ArrayOfStringsSerDe serDe = new ArrayOfStringsSerDe();
private ArrayOfStringsSerDe serDe = new ArrayOfStringsSerDe();

@Test
public void empty() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.apache.datasketches.quantiles;

import java.util.Comparator;

/**
* For testing only
*/
public class ItemsSketchSortedViewString extends ItemsSketchSortedView<String> {

public ItemsSketchSortedViewString(
final String[] quantiles,
final long[] cumWeights,
final long totalN,
final Comparator<String> comparator) {
super(quantiles, cumWeights, totalN, comparator);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public int compare(final String s1, final String s2) {
try {
final int i1 = Integer.parseInt(s1);
final int i2 = Integer.parseInt(s2);
return Integer.valueOf(i1).compareTo(i2);
return Integer.compare(i1,i2);
} catch (NumberFormatException e) {
throw new RuntimeException(e);
}
Expand Down Expand Up @@ -117,7 +117,7 @@ public int compare(final String s1, final String s2) {
try {
final int i1 = Integer.parseInt(s1);
final int i2 = Integer.parseInt(s2);
return Integer.valueOf(i1).compareTo(i2);
return Integer.compare(i1, i2);
} catch (NumberFormatException e) {
throw new RuntimeException(e);
}
Expand Down
Loading

0 comments on commit 2dfa934

Please sign in to comment.