From 844e3b343626fb0461086d9f7171423e30bf946d Mon Sep 17 00:00:00 2001 From: Kevin Smith Date: Fri, 26 Jan 2024 13:50:50 -0800 Subject: [PATCH] Fix warnings and code smells --- .../vdyp/common/ExpectationDifference.java | 2 +- .../bc/gov/nrs/vdyp/common/ValueOrMarker.java | 10 ++----- .../vdyp/io/parse/GenusDefinitionParser.java | 5 ++-- .../gov/nrs/vdyp/model/BaseVdypPolygon.java | 1 - .../gov/nrs/vdyp/model/UtilizationClass.java | 6 ++-- .../ca/bc/gov/nrs/vdyp/model/VdypLayer.java | 1 - .../ca/bc/gov/nrs/vdyp/model/VdypPolygon.java | 4 ++- .../io/parse/BecDefinitionParserTest.java | 7 ----- .../nrs/vdyp/io/parse/ValueParserTest.java | 5 ---- .../io/write/VriAdjustInputWriterTest.java | 5 ---- .../ca/bc/gov/nrs/vdyp/model/FipModeTest.java | 16 +++++------ .../gov/nrs/vdyp/model/VdypPolygonTest.java | 1 - .../gov/nrs/vdyp/model/VdypSpeciesTest.java | 1 - .../java/ca/bc/gov/nrs/vdyp/fip/FipStart.java | 28 ++++++++++--------- .../bc/gov/nrs/vdyp/fip/model/FipLayer.java | 12 ++++++-- .../ca/bc/gov/nrs/vdyp/fip/FipStartTest.java | 1 - .../bc/gov/nrs/vdyp/fip/RootFinderTest.java | 1 - 17 files changed, 44 insertions(+), 62 deletions(-) diff --git a/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/common/ExpectationDifference.java b/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/common/ExpectationDifference.java index 32b455cb5..f360e3b10 100644 --- a/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/common/ExpectationDifference.java +++ b/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/common/ExpectationDifference.java @@ -34,7 +34,7 @@ public static ExpectationDifference difference(Collection values, Coll missing.removeAll(values); var unexpected = new HashSet(values); unexpected.removeAll(expected); - return new ExpectationDifference(missing, unexpected); + return new ExpectationDifference<>(missing, unexpected); } /** diff --git a/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/common/ValueOrMarker.java b/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/common/ValueOrMarker.java index d1e7db6e6..c42aafda3 100644 --- a/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/common/ValueOrMarker.java +++ b/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/common/ValueOrMarker.java @@ -88,7 +88,7 @@ public T handle(Function valueHandler, Function markerH * @return */ public static Builder builder(Class vClazz, Class mClazz) { - return new Builder(); + return new Builder<>(); } /** @@ -102,22 +102,18 @@ public static Builder builder(Class vClazz */ public static class Builder { - public Builder() { - - } - /** * Create a ValueOrMarker with a Marker */ public ValueOrMarker marker(Marker m) { - return new ValueOrMarker(m, true); + return new ValueOrMarker<>(m, true); } /** * Create a ValueOrMarker with a Value */ public ValueOrMarker value(Value v) { - return new ValueOrMarker(v, false); + return new ValueOrMarker<>(v, false); } } } diff --git a/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/io/parse/GenusDefinitionParser.java b/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/io/parse/GenusDefinitionParser.java index ad7d1edf3..6f65209ba 100644 --- a/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/io/parse/GenusDefinitionParser.java +++ b/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/io/parse/GenusDefinitionParser.java @@ -7,7 +7,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.stream.Collectors; import ca.bc.gov.nrs.vdyp.common.Utils; import ca.bc.gov.nrs.vdyp.model.GenusDefinition; @@ -60,9 +59,9 @@ public GenusDefinitionParser() { this.numSp0 = 16; } - public GenusDefinitionParser(int num_sp0) { + public GenusDefinitionParser(int numSp0) { super(); - this.numSp0 = num_sp0; + this.numSp0 = numSp0; } @SuppressWarnings("unchecked") diff --git a/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/model/BaseVdypPolygon.java b/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/model/BaseVdypPolygon.java index e3a16dc97..071d2da86 100644 --- a/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/model/BaseVdypPolygon.java +++ b/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/model/BaseVdypPolygon.java @@ -1,7 +1,6 @@ package ca.bc.gov.nrs.vdyp.model; import java.util.Collection; -import java.util.Collections; import java.util.EnumMap; import java.util.LinkedHashMap; import java.util.LinkedList; diff --git a/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/model/UtilizationClass.java b/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/model/UtilizationClass.java index 8fc9f4a97..9e86cb3bb 100644 --- a/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/model/UtilizationClass.java +++ b/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/model/UtilizationClass.java @@ -11,7 +11,7 @@ public enum UtilizationClass { OVER225(4, ">22.5 cm", 22.5f, 10000f); public final int index; - public final String name; + public final String className; public final float lowBound; public final float highBound; @@ -25,9 +25,9 @@ public enum UtilizationClass { } } - UtilizationClass(int index, String name, float lowBound, float highBound) { + UtilizationClass(int index, String className, float lowBound, float highBound) { this.index = index; - this.name = name; + this.className = className; this.lowBound = lowBound; this.highBound = highBound; } diff --git a/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/model/VdypLayer.java b/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/model/VdypLayer.java index dfac4e924..d2b9d81b1 100644 --- a/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/model/VdypLayer.java +++ b/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/model/VdypLayer.java @@ -1,6 +1,5 @@ package ca.bc.gov.nrs.vdyp.model; -import java.util.Arrays; import java.util.Optional; import java.util.function.Consumer; diff --git a/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/model/VdypPolygon.java b/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/model/VdypPolygon.java index 9f515c300..391c32ae9 100644 --- a/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/model/VdypPolygon.java +++ b/vdyp-common/src/main/java/ca/bc/gov/nrs/vdyp/model/VdypPolygon.java @@ -33,7 +33,9 @@ public , U> VdypPolygon(O toCopy, Function new IllegalArgumentException("Inventory Type Group does not exist if there is no primary layer") + ); } @Computed diff --git a/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/io/parse/BecDefinitionParserTest.java b/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/io/parse/BecDefinitionParserTest.java index 9681c17ff..782b6efd5 100644 --- a/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/io/parse/BecDefinitionParserTest.java +++ b/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/io/parse/BecDefinitionParserTest.java @@ -5,22 +5,15 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.hasProperty; -import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.sameInstance; import static org.junit.jupiter.api.Assertions.assertThrows; -import java.util.ArrayList; import java.util.Collections; -import java.util.List; -import java.util.Map; - import static org.hamcrest.Matchers.equalTo; import org.hamcrest.Matchers; import org.junit.jupiter.api.Test; -import ca.bc.gov.nrs.vdyp.model.BecDefinition; -import ca.bc.gov.nrs.vdyp.model.BecLookup; import ca.bc.gov.nrs.vdyp.model.Region; import ca.bc.gov.nrs.vdyp.test.TestUtils; diff --git a/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/io/parse/ValueParserTest.java b/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/io/parse/ValueParserTest.java index fd2f95f31..f3dd95536 100644 --- a/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/io/parse/ValueParserTest.java +++ b/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/io/parse/ValueParserTest.java @@ -1,13 +1,9 @@ package ca.bc.gov.nrs.vdyp.io.parse; -import static ca.bc.gov.nrs.vdyp.common.Utils.constMap; -import static ca.bc.gov.nrs.vdyp.test.VdypMatchers.isMarker; -import static ca.bc.gov.nrs.vdyp.test.VdypMatchers.isValue; import static ca.bc.gov.nrs.vdyp.test.VdypMatchers.notPresent; import static ca.bc.gov.nrs.vdyp.test.VdypMatchers.present; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.aMapWithSize; -import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasProperty; import static org.hamcrest.Matchers.is; @@ -19,7 +15,6 @@ import org.junit.jupiter.api.Test; -import ca.bc.gov.nrs.vdyp.common.Utils; import ca.bc.gov.nrs.vdyp.model.LayerType; class ValueParserTest { diff --git a/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/io/write/VriAdjustInputWriterTest.java b/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/io/write/VriAdjustInputWriterTest.java index 9d236c73b..b9ac95e60 100644 --- a/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/io/write/VriAdjustInputWriterTest.java +++ b/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/io/write/VriAdjustInputWriterTest.java @@ -7,19 +7,14 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.util.Collections; import java.util.HashMap; -import java.util.List; import java.util.Map; -import java.util.Optional; - import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import ca.bc.gov.nrs.vdyp.common.ControlKeys; import ca.bc.gov.nrs.vdyp.common.Utils; import ca.bc.gov.nrs.vdyp.io.FileResolver; -import ca.bc.gov.nrs.vdyp.model.Coefficients; import ca.bc.gov.nrs.vdyp.model.FipMode; import ca.bc.gov.nrs.vdyp.model.LayerType; import ca.bc.gov.nrs.vdyp.model.VdypLayer; diff --git a/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/model/FipModeTest.java b/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/model/FipModeTest.java index 4188b27f8..f0a7917af 100644 --- a/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/model/FipModeTest.java +++ b/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/model/FipModeTest.java @@ -12,22 +12,22 @@ class FipModeTest { @ParameterizedTest() - @EnumSource(FipMode.class) - void testGetByCodeExpected(FipMode mode){ + @EnumSource(FipMode.class) + void testGetByCodeExpected(FipMode mode) { var result = FipMode.getByCode(mode.getCode()); assertThat(result, present(is(mode))); } - + @ParameterizedTest() - @ValueSource(ints= {-2, Integer.MIN_VALUE, Integer.MAX_VALUE, 42}) - void testGetByCodeUnexpected(int code){ + @ValueSource(ints = { -2, Integer.MIN_VALUE, Integer.MAX_VALUE, 42 }) + void testGetByCodeUnexpected(int code) { var result = FipMode.getByCode(code); assertThat(result, present(is(FipMode.DONT_PROCESS))); } - + @ParameterizedTest() - @ValueSource(ints= {0}) - void testGetByCodeMissing(int code){ + @ValueSource(ints = { 0 }) + void testGetByCodeMissing(int code) { var result = FipMode.getByCode(code); assertThat(result, notPresent()); } diff --git a/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/model/VdypPolygonTest.java b/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/model/VdypPolygonTest.java index d7958c1e0..2882a1e40 100644 --- a/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/model/VdypPolygonTest.java +++ b/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/model/VdypPolygonTest.java @@ -5,7 +5,6 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import org.easymock.EasyMock; -import org.hamcrest.Matchers; import org.junit.jupiter.api.Test; class VdypPolygonTest { diff --git a/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/model/VdypSpeciesTest.java b/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/model/VdypSpeciesTest.java index 4ac1dae45..d4097dde1 100644 --- a/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/model/VdypSpeciesTest.java +++ b/vdyp-common/src/test/java/ca/bc/gov/nrs/vdyp/model/VdypSpeciesTest.java @@ -4,7 +4,6 @@ import static org.hamcrest.Matchers.*; import static org.junit.jupiter.api.Assertions.assertThrows; -import org.easymock.EasyMock; import org.junit.jupiter.api.Test; class VdypSpeciesTest { diff --git a/vdyp-fip/src/main/java/ca/bc/gov/nrs/vdyp/fip/FipStart.java b/vdyp-fip/src/main/java/ca/bc/gov/nrs/vdyp/fip/FipStart.java index 26b43aa70..baa549804 100644 --- a/vdyp-fip/src/main/java/ca/bc/gov/nrs/vdyp/fip/FipStart.java +++ b/vdyp-fip/src/main/java/ca/bc/gov/nrs/vdyp/fip/FipStart.java @@ -366,14 +366,13 @@ VdypPolygon createVdypPolygon(FipPolygon fipPolygon, Map p float percentAvailable = estimatePercentForestLand(fipPolygon, fipVetLayer, fipPrimaryLayer); - var vdypPolygon = VdypPolygon.build(builder -> { - builder.copy(fipPolygon, (x) -> percentAvailable); - }); + var vdypPolygon = VdypPolygon.build(builder -> builder.copy(fipPolygon, x -> percentAvailable)); vdypPolygon.setLayers(processedLayers); return vdypPolygon; } // FIPLAND + @SuppressWarnings("java:S3655") float estimatePercentForestLand(FipPolygon fipPolygon, FipLayer fipVetLayer, FipLayerPrimary fipPrimaryLayer) throws ProcessingException { if (fipPolygon.getPercentAvailable().isPresent()) { @@ -1575,6 +1574,7 @@ void reconcileComponents( private static final List MODE_1_RECONCILE_AVAILABILITY_CLASSES = List .of(UtilizationClass.OVER225, UtilizationClass.U175TO225, UtilizationClass.U125TO175); + @SuppressWarnings("java:S3655") void reconcileComponentsMode1( Coefficients baseAreaUtil, Coefficients treesPerHectareUtil, Coefficients quadMeanDiameterUtil, float tphSumHigh @@ -1741,6 +1741,7 @@ private void reconcileComponentsMode2( } } + @SuppressWarnings("java:S3655") void reconcileComponentsMode3( Coefficients baseAreaUtil, Coefficients treesPerHectareUtil, Coefficients quadMeanDiameterUtil ) { @@ -1825,7 +1826,7 @@ void estimateQuadMeanDiameterByUtilization(BecDefinition bec, Coefficients quadM float quadMeanDiameter07 = quadMeanDiameterUtil.getCoe(UTIL_ALL); for (var uc : UTIL_CLASSES) { - log.atDebug().setMessage("For util level {}").addArgument(uc.name); + log.atDebug().setMessage("For util level {}").addArgument(uc.className); final var coeMap = Utils.>expectParsedControl( controlMap, UtilComponentDQParser.CONTROL_KEY, MatrixMap3.class ); @@ -1876,15 +1877,17 @@ void estimateQuadMeanDiameterByUtilization(BecDefinition bec, Coefficients quadM throw new IllegalStateException( "Should not be attempting to process small component or all large components" ); + default: + throw new IllegalStateException("Unknown utilization class " + uc); } - log.atDebug().setMessage("Util DQ for class {} is {}").addArgument(uc.name) + log.atDebug().setMessage("Util DQ for class {} is {}").addArgument(uc.className) .addArgument(quadMeanDiameterUtil.getCoe(uc.index)); } log.atTrace().setMessage("Estimated Diameters {}").addArgument( () -> UTIL_CLASSES.stream() - .map(uc -> String.format("%s: %d", uc.name, quadMeanDiameterUtil.getCoe(uc.index))) + .map(uc -> String.format("%s: %d", uc.className, quadMeanDiameterUtil.getCoe(uc.index))) ); } @@ -1915,15 +1918,14 @@ List findPrimarySpecies(Map allSpecies) { // Start with a deep copy of the species map so there are no side effects from // the manipulation this method does. var combined = new HashMap(allSpecies.size()); - allSpecies.entrySet().stream().forEach(spec -> { - combined.put(spec.getKey(), new FipSpecies(spec.getValue())); - }); + allSpecies.entrySet().stream().forEach(spec -> combined.put(spec.getKey(), new FipSpecies(spec.getValue()))); for (var combinationGroup : PRIMARY_SPECIES_TO_COMBINE) { var groupSpecies = combinationGroup.stream().map(combined::get).filter(Objects::nonNull).toList(); if (groupSpecies.size() < 2) { continue; } + @SuppressWarnings("java:S3655") var groupPrimary = new FipSpecies(groupSpecies.stream().sorted(PERCENT_GENUS_DESCENDING).findFirst().get()); var total = (float) groupSpecies.stream().mapToDouble(FipSpecies::getPercentGenus).sum(); combinationGroup.forEach(combined::remove); @@ -2522,9 +2524,7 @@ float normalizeUtilizationComponents(Coefficients components) throws ProcessingE if (sum <= 0f) { throw new ProcessingException("Total volume " + sum + " was not positive."); } - UTIL_CLASSES.forEach(uc -> { - components.setCoe(uc.index, components.getCoe(uc.index) * k); - }); + UTIL_CLASSES.forEach(uc -> components.setCoe(uc.index, components.getCoe(uc.index) * k)); return k; } @@ -3052,7 +3052,9 @@ public void estimateSmallComponents(FipPolygon fPoly, VdypLayer layer) { float treesPerHectareSum = 0f; float volumeSum = 0f; - Region region = BecDefinitionParser.getBecs(controlMap).get(fPoly.getBiogeoclimaticZone()).get().getRegion(); + Region region = BecDefinitionParser.getBecs(controlMap).get(fPoly.getBiogeoclimaticZone()) + .orElseThrow(() -> new IllegalStateException("Could not find BEC " + fPoly.getBiogeoclimaticZone())) + .getRegion(); for (VdypSpecies spec : layer.getSpecies().values()) { float loreyHeightSpec = spec.getLoreyHeightByUtilization().getCoe(UTIL_ALL); // HLsp diff --git a/vdyp-fip/src/main/java/ca/bc/gov/nrs/vdyp/fip/model/FipLayer.java b/vdyp-fip/src/main/java/ca/bc/gov/nrs/vdyp/fip/model/FipLayer.java index 4521f445b..8c648e4d1 100644 --- a/vdyp-fip/src/main/java/ca/bc/gov/nrs/vdyp/fip/model/FipLayer.java +++ b/vdyp-fip/src/main/java/ca/bc/gov/nrs/vdyp/fip/model/FipLayer.java @@ -74,19 +74,25 @@ public void setYearsToBreastHeightSafe(float yearsToBreastHeight) { @Override public void setAgeTotal(Optional ageTotal) { - ageTotal.orElseThrow(() -> new IllegalArgumentException()); + if(ageTotal.isEmpty()) { + throw new IllegalArgumentException("ageTotal must not be empty"); + } super.setAgeTotal(ageTotal); } @Override public void setHeight(Optional height) { - height.orElseThrow(() -> new IllegalArgumentException()); + if(height.isEmpty()) { + throw new IllegalArgumentException("height must not be empty"); + } super.setHeight(height); } @Override public void setYearsToBreastHeight(Optional yearsToBreastHeight) { - yearsToBreastHeight.orElseThrow(() -> new IllegalArgumentException()); + if(yearsToBreastHeight.isEmpty()) { + throw new IllegalArgumentException("yearsToBreastHeight must not be empty"); + } super.setYearsToBreastHeight(yearsToBreastHeight); } diff --git a/vdyp-fip/src/test/java/ca/bc/gov/nrs/vdyp/fip/FipStartTest.java b/vdyp-fip/src/test/java/ca/bc/gov/nrs/vdyp/fip/FipStartTest.java index f9052d401..69c58a312 100644 --- a/vdyp-fip/src/test/java/ca/bc/gov/nrs/vdyp/fip/FipStartTest.java +++ b/vdyp-fip/src/test/java/ca/bc/gov/nrs/vdyp/fip/FipStartTest.java @@ -38,7 +38,6 @@ import org.easymock.EasyMock; import org.easymock.IMocksControl; -import org.easymock.bytebuddy.agent.builder.AgentBuilder; import org.hamcrest.Description; import org.hamcrest.Matcher; import org.hamcrest.Matchers; diff --git a/vdyp-fip/src/test/java/ca/bc/gov/nrs/vdyp/fip/RootFinderTest.java b/vdyp-fip/src/test/java/ca/bc/gov/nrs/vdyp/fip/RootFinderTest.java index 92753f4cb..88ed9e013 100644 --- a/vdyp-fip/src/test/java/ca/bc/gov/nrs/vdyp/fip/RootFinderTest.java +++ b/vdyp-fip/src/test/java/ca/bc/gov/nrs/vdyp/fip/RootFinderTest.java @@ -5,7 +5,6 @@ import static org.hamcrest.Matchers.contains; import java.util.Arrays; -import java.util.LinkedHashMap; import java.util.Map; import org.apache.commons.math3.analysis.MultivariateMatrixFunction; import org.apache.commons.math3.analysis.MultivariateVectorFunction;