Refactored some implementation and design code smells

base/src/main/java/com/acadiasoft/im/base/margin/BasicImTree.java

@@ -60,9 +60,13 @@ public BasicImTree(ImTree<? extends MarginIdentifier, ? extends ImTree> tree) {
     this.level = tree.getTreeLevel();
     this.margin = tree.getMargin();
     this.identifier = tree.getMarginIdentifier().getMarginIdentifier();
-    this.children = tree.getChildren().stream()
-        // FIXME: why does compiler require this cast??? should be redundant...
-        .map(t -> new BasicImTree((ImTree<MarginIdentifier, ImTree>) t)).collect(Collectors.toList());
+    this.children = transformChildren(tree.getChildren());
+  }
+
+  private List<BasicImTree> transformChildren(List<? extends ImTree> children) {
+    return children.stream()
+            .map(child -> new BasicImTree((ImTree<MarginIdentifier, ImTree>) child))
+            .collect(Collectors.toList());
   }
 
   @Override

base/src/main/java/com/acadiasoft/im/base/margin/BlankImTree.java

@@ -32,6 +32,7 @@
 public class BlankImTree extends TotalMargin {
 
   private static final long serialVersionUID = 1L;
+  private static final BlankImTree instance = new BlankImTree();
 
   private BlankImTree() {
     super(BigDecimal.ZERO, Collections.emptyList());
@@ -41,4 +42,8 @@ public static BlankImTree buildTree() {
     return new BlankImTree();
   }
 
+  public static BlankImTree getInstance() {
+    return instance;
+  }
+
 }

base/src/main/java/com/acadiasoft/im/base/margin/TotalMargin.java

@@ -27,8 +27,6 @@
 
 import java.math.BigDecimal;
 import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
 
 public class TotalMargin implements ImTree<TotalClass, ModelMargin> {
@@ -43,25 +41,6 @@ protected TotalMargin(BigDecimal margin, List<ModelMargin> children) {
     this.children.addAll(children);
   }
 
-  public static TotalMargin build(ModelMargin child) {
-    if (child.isMarginZero()) {
-      return BlankImTree.buildTree();
-    } else {
-      return new TotalMargin(child.getMargin(), Collections.singletonList(child));
-    }
-  }
-
-  public static TotalMargin build(ModelMargin one, ModelMargin two) {
-    if (one.isMarginZero()) {
-      return TotalMargin.build(two);
-    } else if (two.isMarginZero()) {
-      return TotalMargin.build(one);
-    } else {
-      BigDecimal sum = one.getMargin().add(two.getMargin());
-      return new TotalMargin(sum, Arrays.asList(one, two));
-    }
-  }
-
   @Override
   public TotalClass getMarginIdentifier() {
     return TotalClass.TOTAL;

base/src/main/java/com/acadiasoft/im/base/margin/TotalMarginFactory.java

@@ -0,0 +1,27 @@
+package com.acadiasoft.im.base.margin;
+
+import java.math.BigDecimal;
+import java.util.Arrays;
+import java.util.Collections;
+
+public class TotalMarginFactory {
+
+    public static TotalMargin build(ModelMargin child) {
+        if (child.isMarginZero()) {
+            return BlankImTree.getInstance();
+        } else {
+            return new TotalMargin(child.getMargin(), Collections.singletonList(child));
+        }
+    }
+
+    public static TotalMargin build(ModelMargin one, ModelMargin two) {
+        if (one.isMarginZero()) {
+            return build(two);
+        } else if (two.isMarginZero()) {
+            return build(one);
+        } else {
+            BigDecimal sum = one.getMargin().add(two.getMargin());
+            return new TotalMargin(sum, Arrays.asList(one, two));
+        }
+    }
+}

base/src/test/java/com/acadiasoft/im/base/model/BasicImTreeSerializationIT.java

@@ -22,6 +22,7 @@
 
 package com.acadiasoft.im.base.model;
 
+import com.acadiasoft.im.base.fx.NoConversionFxRate;
 import com.acadiasoft.im.base.margin.BasicImTree;
 import com.acadiasoft.im.base.margin.ModelMargin;
 import com.acadiasoft.im.base.margin.SiloMargin;
@@ -30,16 +31,90 @@
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.junit.Assert;
+import org.junit.Before;
 import org.junit.Test;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
 
 import java.io.IOException;
 import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
+import static org.mockito.Mockito.*;
+
 public class BasicImTreeSerializationIT {
 
+  @Mock
+  private ModelMargin mockMarginOne;
+
+  @Mock
+  private ModelMargin mockMarginTwo;
+
+  @Before
+  public void setUp() {
+    MockitoAnnotations.initMocks(this);
+  }
+
+  @Test
+  public void testBuildWithNonZeroMargins() {
+    //Arrange
+    when(mockMarginOne.isMarginZero()).thenReturn(false);
+    when(mockMarginOne.getMargin()).thenReturn(new BigDecimal("100"));
+    when(mockMarginTwo.isMarginZero()).thenReturn(false);
+    when(mockMarginTwo.getMargin()).thenReturn(new BigDecimal("200"));
+
+    //Act
+    TotalMargin result = TotalMargin.build(mockMarginOne, mockMarginTwo);
+
+    //Assert
+    Assert.assertEquals(new BigDecimal("300"), result.getMargin());
+    Assert.assertEquals(2, result.getChildren().size());
+  }
+
+  @Test
+  public void testBuildWithOneZeroMargin() {
+    //Arrange
+    when(mockMarginOne.isMarginZero()).thenReturn(true);
+    when(mockMarginTwo.isMarginZero()).thenReturn(false);
+    when(mockMarginTwo.getMargin()).thenReturn(new BigDecimal("200"));
+
+    //Act
+    TotalMargin result = TotalMargin.build(mockMarginOne, mockMarginTwo);
+
+    //Assert
+    Assert.assertEquals(new BigDecimal("200"), result.getMargin());
+  }
+
+  @Test
+  public void testBuildWithBothZeroMargins() {
+    //Arrange
+    when(mockMarginOne.isMarginZero()).thenReturn(true);
+    when(mockMarginTwo.isMarginZero()).thenReturn(true);
+
+    //Act
+    TotalMargin result = TotalMargin.build(mockMarginOne, mockMarginTwo);
+
+    //Assert
+    Assert.assertEquals(0, result.getMargin().compareTo(BigDecimal.ZERO));
+  }
+
+  @Test
+  public void testConvert() {
+    //Arrange
+    NoConversionFxRate fxRate = new NoConversionFxRate();
+    BigDecimal amount = new BigDecimal("100.00");
+    String fromCurrency = "USD";
+    String toCurrency = "EUR";
+
+    //Act
+    BigDecimal convertedAmount = fxRate.convert(amount, fromCurrency, toCurrency);
+
+    //Assert
+    Assert.assertEquals(0, convertedAmount.compareTo(amount));
+  }
+
   @Test
   public void testSerialization() throws JsonProcessingException {
     BasicImTree one = new BasicImTree("2.Whatever", "hello", new BigDecimal("3.0"), new ArrayList<>());

pom.xml

@@ -53,6 +53,14 @@
       <scope>test</scope>
     </dependency>
 
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-core</artifactId>
+      <version>3.3.3</version>
+      <scope>test</scope>
+    </dependency>
+
+
     <dependency>
       <groupId>commons-io</groupId>
       <artifactId>commons-io</artifactId>

schedule/src/main/java/com/acadiasoft/im/schedule/models/parameters/Parameters.java

@@ -26,6 +26,10 @@
 
 import java.math.BigDecimal;
 import java.time.LocalDate;
+import java.time.temporal.ChronoUnit;
+
+import static com.acadiasoft.im.schedule.models.imtree.identifiers.ScheduleProductClass.CREDIT;
+import static com.acadiasoft.im.schedule.models.imtree.identifiers.ScheduleProductClass.RATES;
 
 /**
  *
@@ -48,33 +52,41 @@ public class Parameters implements ScheduleParameters {
 
   @Override
   public BigDecimal getParameter(ScheduleProductClass productClass, LocalDate val, LocalDate end) {
-    if (productClass.equals(ScheduleProductClass.RATES)) {
-      if (end.minusYears(TWO_YEARS).isBefore(val) || end.minusYears(TWO_YEARS).isEqual(val)) {
-        return RATES_UNDER_2_YEARS;
-      } else if (end.minusYears(FIVE_YEARS).isBefore(val) || end.minusYears(FIVE_YEARS).isEqual(val)) {
-        return RATES_2_TO_5_YEARS;
-      } else {
-        return RATES_OVER_5_YEARS;
-      }
-    } else if (productClass.equals(ScheduleProductClass.CREDIT)) {
-      if (end.minusYears(TWO_YEARS).isBefore(val) || end.minusYears(TWO_YEARS).isEqual(val)) {
-        return CREDIT_UNDER_2_YEARS;
-      } else if (end.minusYears(FIVE_YEARS).isBefore(val) || end.minusYears(FIVE_YEARS).isEqual(val)) {
-        return CREDIT_2_TO_5_YEARS;
-      } else {
-        return CREDIT_OVER_5_YEARS;
-      }
-    } else if (productClass.equals(ScheduleProductClass.FX)) {
-      return FX;
-    } else if (productClass.equals(ScheduleProductClass.EQUITY)) {
-      return EQUITY;
-    } else if (productClass.equals(ScheduleProductClass.COMMODITY)) {
-      return COMMODITY;
-    } else if (productClass.equals(ScheduleProductClass.OTHER)) {
-      return OTHER;
-    } else {
-      // Should never get here
+      if (productClass.equals(RATES)) {
+          return getRatesParameter(val, end);
+      } else if (productClass.equals(CREDIT)) {
+          return getCreditParameter(val, end);
+      } else if (productClass.equals(FX)) {
+          return FX;
+      } else if (productClass.equals(EQUITY)) {
+          return EQUITY;
+      } else if (productClass.equals(COMMODITY)) {
+          return COMMODITY;
+      } else if (productClass.equals(OTHER)) {
+          return OTHER;
+      }// Should never get here
       return null;
+  }
+
+  private BigDecimal getRatesParameter(LocalDate val, LocalDate end) {
+    long yearsBetween = ChronoUnit.YEARS.between(val, end);
+    if (yearsBetween <= TWO_YEARS) {
+      return RATES_UNDER_2_YEARS;
+    } else if (yearsBetween <= FIVE_YEARS) {
+      return RATES_2_TO_5_YEARS;
+    } else {
+      return RATES_OVER_5_YEARS;
+    }
+  }
+
+  private BigDecimal getCreditParameter(LocalDate val, LocalDate end) {
+    long yearsBetween = ChronoUnit.YEARS.between(val, end);
+    if (yearsBetween <= TWO_YEARS) {
+      return CREDIT_UNDER_2_YEARS;
+    } else if (yearsBetween <= FIVE_YEARS) {
+      return CREDIT_2_TO_5_YEARS;
+    } else {
+      return CREDIT_OVER_5_YEARS;
     }
   }
 

schedule/src/test/java/com/acadiasoft/im/ScheduleTest.java

@@ -23,7 +23,9 @@
 package com.acadiasoft.im;
 
 import com.acadiasoft.im.base.fx.FxRate;
+import com.acadiasoft.im.schedule.config.DefaultScheduleConfigBuilder;
 import com.acadiasoft.im.schedule.config.ScheduleConfig;
+import com.acadiasoft.im.schedule.config.ScheduleConfigBuilder;
 import com.acadiasoft.im.schedule.engine.Schedule;
 import com.acadiasoft.im.schedule.models.DefaultScheduleSensitivity;
 import com.acadiasoft.im.schedule.models.ScheduleNotional;
@@ -36,12 +38,40 @@
 import java.math.RoundingMode;
 import java.util.Arrays;
 
+import static org.mockito.Mockito.mock;
+
 /**
  *
  * @author alec.stewart
  */
 public class ScheduleTest {
 
+  @Test
+  public void testFxRateNonNull() {
+    //Arrange
+    ScheduleConfigBuilder builder = new DefaultScheduleConfigBuilder();
+    FxRate newFxRate = mock(FxRate.class);
+
+    //Act
+    builder.fxRate(newFxRate);
+
+    //Assert
+    Assert.assertEquals(newFxRate, builder.fxRate());
+  }
+
+  @Test
+  public void testFxRateNull() {
+    //Arrange
+    ScheduleConfigBuilder builder = new DefaultScheduleConfigBuilder();
+    FxRate initialFxRate = builder.fxRate();
+
+    //Act
+    builder.fxRate(null);
+
+    //Assert
+    Assert.assertEquals(initialFxRate, builder.fxRate());
+  }
+
   @Test
   public void testNettingWorksAsExpected() {
     ScheduleNotional one = new DefaultScheduleSensitivity("trade1", "Rates", "Notional", "2018-09-12", "2018-11-23", "1000", FxRate.USD, "1000");

simm/src/main/java/com/acadiasoft/im/simm/model/SensitivityIdentifier.java

@@ -55,15 +55,37 @@ public interface SensitivityIdentifier extends Serializable {
   String BAD_PRODUCT = "The product class was null or empty when it was required";
 
   default void checkSensitivityIdentifier(String productClass, String riskType, String qualifier, String bucket, String label1, String label2) {
-    if (riskType == null || riskType.isEmpty()) {
+    checkRiskType(riskType);
+    checkQualifier(qualifier, riskType);
+    checkProductClass(productClass);
+  }
+
+  default void checkRiskType(String riskType) {
+    if (isInvalid(riskType)) {
       throw new IllegalStateException(BAD_RISK_TYPE);
-    } else if ((qualifier == null || qualifier.isEmpty()) && !riskType.equalsIgnoreCase(AddOnSubType.ADD_ON_FIXED_AMOUNT)) {
+    }
+  }
+
+  default void checkQualifier(String qualifier, String riskType) {
+    if (isInvalid(qualifier) && !isAddOnFixedAmount(riskType)) {
       throw new IllegalStateException(BAD_QUALIFIER);
-    } else if (isSimmStandard() && (productClass == null || productClass.isEmpty())) {
+    }
+  }
+
+  default void checkProductClass(String productClass) {
+    if (isSimmStandard() && isInvalid(productClass)) {
       throw new IllegalStateException(BAD_PRODUCT);
     }
   }
 
+  default boolean isInvalid(String value) {
+    return value == null || value.isEmpty();
+  }
+
+  default boolean isAddOnFixedAmount(String riskType) {
+    return riskType.equalsIgnoreCase(AddOnSubType.ADD_ON_FIXED_AMOUNT);
+  }
+
   default boolean isAddOn() {
     return AddOnSubType.isAddOnSubType(this.getRiskType());
   }

simm/src/main/java/com/acadiasoft/im/simm/model/param/BaseCorrRiskStrategy.java

@@ -0,0 +1,9 @@
+package com.acadiasoft.im.simm.model.param;
+
+import com.acadiasoft.im.simm.model.imtree.identifiers.WeightingClass;
+import java.math.BigDecimal;
+
+public interface BaseCorrRiskStrategy {
+    BigDecimal getRiskWeight(WeightingClass w);
+    BigDecimal getSensitivityCorrelation(WeightingClass r, WeightingClass s);
+}