Skip to content

Commit

Permalink
Fix warnings and code smells
Browse files Browse the repository at this point in the history
  • Loading branch information
smithkm committed Jan 26, 2024
1 parent ebb1080 commit 844e3b3
Show file tree
Hide file tree
Showing 17 changed files with 44 additions and 62 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public static <U> ExpectationDifference<U> difference(Collection<U> values, Coll
missing.removeAll(values);
var unexpected = new HashSet<U>(values);
unexpected.removeAll(expected);
return new ExpectationDifference<U>(missing, unexpected);
return new ExpectationDifference<>(missing, unexpected);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public <T> T handle(Function<Value, T> valueHandler, Function<Marker, T> markerH
* @return
*/
public static <Value, Marker> Builder<Value, Marker> builder(Class<Value> vClazz, Class<Marker> mClazz) {
return new Builder<Value, Marker>();
return new Builder<>();
}

/**
Expand All @@ -102,22 +102,18 @@ public static <Value, Marker> Builder<Value, Marker> builder(Class<Value> vClazz
*/
public static class Builder<Value, Marker> {

public Builder() {

}

/**
* Create a ValueOrMarker with a Marker
*/
public ValueOrMarker<Value, Marker> marker(Marker m) {
return new ValueOrMarker<Value, Marker>(m, true);
return new ValueOrMarker<>(m, true);
}

/**
* Create a ValueOrMarker with a Value
*/
public ValueOrMarker<Value, Marker> value(Value v) {
return new ValueOrMarker<Value, Marker>(v, false);
return new ValueOrMarker<>(v, false);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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")
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package ca.bc.gov.nrs.vdyp.model;

import java.util.Arrays;
import java.util.Optional;
import java.util.function.Consumer;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ public <O extends BaseVdypPolygon<?, U>, U> VdypPolygon(O toCopy, Function<U, Fl

@Computed
public int getInventoryTypeGroup() {
return this.getLayers().get(LayerType.PRIMARY).getInventoryTypeGroup().get();
return this.getLayers().get(LayerType.PRIMARY).getInventoryTypeGroup().orElseThrow(
() -> new IllegalArgumentException("Inventory Type Group does not exist if there is no primary layer")
);
}

@Computed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
28 changes: 15 additions & 13 deletions vdyp-fip/src/main/java/ca/bc/gov/nrs/vdyp/fip/FipStart.java
Original file line number Diff line number Diff line change
Expand Up @@ -366,14 +366,13 @@ VdypPolygon createVdypPolygon(FipPolygon fipPolygon, Map<LayerType, VdypLayer> 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()) {
Expand Down Expand Up @@ -1575,6 +1574,7 @@ void reconcileComponents(
private static final List<UtilizationClass> 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
Expand Down Expand Up @@ -1741,6 +1741,7 @@ private void reconcileComponentsMode2(
}
}

@SuppressWarnings("java:S3655")
void reconcileComponentsMode3(
Coefficients baseAreaUtil, Coefficients treesPerHectareUtil, Coefficients quadMeanDiameterUtil
) {
Expand Down Expand Up @@ -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.<MatrixMap3<Integer, String, String, Coefficients>>expectParsedControl(
controlMap, UtilComponentDQParser.CONTROL_KEY, MatrixMap3.class
);
Expand Down Expand Up @@ -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)))
);

}
Expand Down Expand Up @@ -1915,15 +1918,14 @@ List<FipSpecies> findPrimarySpecies(Map<String, FipSpecies> 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<String, FipSpecies>(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);
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,25 @@ public void setYearsToBreastHeightSafe(float yearsToBreastHeight) {

@Override
public void setAgeTotal(Optional<Float> ageTotal) {
ageTotal.orElseThrow(() -> new IllegalArgumentException());
if(ageTotal.isEmpty()) {
throw new IllegalArgumentException("ageTotal must not be empty");
}
super.setAgeTotal(ageTotal);
}

@Override
public void setHeight(Optional<Float> height) {
height.orElseThrow(() -> new IllegalArgumentException());
if(height.isEmpty()) {
throw new IllegalArgumentException("height must not be empty");
}
super.setHeight(height);
}

@Override
public void setYearsToBreastHeight(Optional<Float> yearsToBreastHeight) {
yearsToBreastHeight.orElseThrow(() -> new IllegalArgumentException());
if(yearsToBreastHeight.isEmpty()) {
throw new IllegalArgumentException("yearsToBreastHeight must not be empty");
}
super.setYearsToBreastHeight(yearsToBreastHeight);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 844e3b3

Please sign in to comment.