VDYP-206 FIPStart output to VRIAdjust input format #26
Conversation
smithkm
changed the title
|
SonarCloud Quality Gate failed.
|
smithkm
force-pushed
the
feature/VDYP-206
branch
4 times, most recently
from
4c14955 to
350c81d
Compare
smithkm
force-pushed
the
feature/VDYP-206
branch
from
350c81d to
844e3b3
Compare
|
|
|
||
| public BaseVdypLayer(String polygonIdentifier, Layer layer) { | ||
| super(); | ||
| this.polygonIdentifier = polygonIdentifier; |
There was a problem hiding this comment.
Given that polygonIdentifier and layer are not Optionals, I'm assuming that these arguments can't be null. Should an IllegalArgumentException be thrown if they are?
| String polygonIdentifier, PA percentAvailable, String fiz, String becIdentifier, Optional<FipMode> modeFip | ||
| ) { | ||
| super(); | ||
| this.forestInventoryZone = fiz; |
There was a problem hiding this comment.
If the non-Optionals can't be null, throw an IllegalArgumentException here when they are.
| return polygonIdentifier; | ||
| } | ||
|
|
||
| public void setPolygonIdentifier(String polygonIdentifier) { |
|
|
||
| Map<String, Float> speciesPercent; // Map from | ||
|
|
||
| protected BaseVdypSpecies(String polygonIdentifier, Layer layer, String genus) { |
There was a problem hiding this comment.
Add throws of IllegalArgumentException when fields that are not nullable (e.g. polygonIdentifier?) are supplied as null.
| } | ||
| } | ||
|
|
||
| private final Map<String, BecDefinition> becMap; |
There was a problem hiding this comment.
Standard practice is for fields to be listed before the method definitions.
| return baseBec; | ||
| } | ||
| return new BecDefinition( | ||
| baseBec, defaultBec, !NON_GROWTH_BECS.contains(baseBec.getAlias()), |
There was a problem hiding this comment.
Use isGrowth, etc, from above.
| public static final String CONTROL_KEY = "BREAKAGE"; | ||
|
|
||
| public BreakageParser() { | ||
| super(Integer.class, 1, CONTROL_KEY); | ||
| this.groupIndexKey(40); | ||
| this.groupIndexKey(MAX_GROUPS); | ||
| this.coefficients(6, 9); |
There was a problem hiding this comment.
Why "6"? There's only 4 coordinates per line
|
|
||
| public CloseUtilVolumeParser() { | ||
| super(1, CONTROL_KEY); | ||
| this.ucIndexKey().space(1).groupIndexKey(MAX_GROUPS).coefficients(4, 10); |
There was a problem hiding this comment.
Why "4"? There's only three coefficients...
| @@ -16,7 +15,7 @@ | |||
| * @author Kevin Smith, Vivid Solutions | |||
| * | |||
| */ | |||
| public class BySpeciesDqCoefficientParser implements ControlMapSubResourceParser<List<Coefficients>> { | |||
There was a problem hiding this comment.
See comments in Word document
| public static final float PI_40K = 0.78539816E-04f; | ||
|
|
||
| // FT_BD | ||
| public static float treesPerHectare(float baseArea, float quadraticMeanDiameter) { |
There was a problem hiding this comment.
We should adopt a naming convention wherein units are always suffixed to variable names. In this case, "baseArea" is a number, but what are the units? It's apparent that they're hectares but this should be made explicit. For time, I always use _ms = milliseconds, _s = seconds, and _m, _h, _d, _w, _y. For area, I suggest _h for hectares. I'm not sure what other ones are used in the system. Suggestions?
Remove keep_versions for initial deployment
Do not review until #25 is merged