Merge pull request #634 from ie3-institute/sp/#633-fix-code-smells

Fixing some code smells

Author: danielfeismann

CHANGELOG.md

@@ -14,7 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `TemperatureDependantLoadProfile`s for depiction of profile behavior of night storage heating and heat pumps [#601](https://github.com/ie3-institute/PowerSystemDataModel/issues/601)
 - `ThermalUnits` as a container to hold all thermal units [#134](https://github.com/ie3-institute/PowerSystemDataModel/issues/134)
 - `ThermalInput` as a distinct abstract class for all thermal models
-- `ThermalGrid` as a container for a completly connected thermal grid
+- `ThermalGrid` as a container for a completely connected thermal grid
 
 ### Fixed
 - Reduced code smells [#492](https://github.com/ie3-institute/PowerSystemDataModel/issues/492)
@@ -25,6 +25,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
     - Replace lambdas with method references
     - Use `Stream#toList`
     - Adapt visibility for JUnit 5
+- More code smell fixing [#633](https://github.com/ie3-institute/PowerSystemDataModel/issues/633)
+    - Use `List#of`
+    - Use direct assignment with switch/case structures
+    - Turn some classes into records
+    - Making abstract classes' constructor protected
+    - Improving some RegExs
+    - Replacing `filter(Optional::isPresent).map(Optional::get)` on streams with `flatMap(Optional::stream)`
+    - instanceof variable declarations
+    - Removing unnecessary parentheses
+    - Miscellaneous code smells
 - Fix JavaDoc creation
     - Create JavaDoc with java 17 instead of java 8
     - Let JavDoc pass, if there are warnings **ATTENTION:** Should be removed, when JavaDoc is fixed! (cf. Issue [#494](https://github.com/ie3-institute/PowerSystemDataModel/issues/494))

src/main/java/edu/ie3/datamodel/graph/SubGridGate.java

@@ -12,13 +12,13 @@
 import edu.ie3.datamodel.models.input.connector.TransformerInput;
 import edu.ie3.datamodel.models.input.container.SubGridContainer;
 import java.io.Serializable;
-import java.util.Objects;
 
 /**
  * Defines gates between {@link SubGridContainer}s and serves as edge definition for {@link
  * SubGridTopologyGraph}
  */
-public class SubGridGate implements Serializable {
+public record SubGridGate(TransformerInput link, NodeInput superiorNode, NodeInput inferiorNode)
+    implements Serializable {
   /**
    * Creates a sub grid gate from two winding transformer.
    *
@@ -40,47 +40,33 @@ public static SubGridGate fromTransformer2W(Transformer2WInput transformer) {
    */
   public static SubGridGate fromTransformer3W(
       Transformer3WInput transformer, ConnectorPort inferiorPort) {
-    switch (inferiorPort) {
-      case B:
-        return new SubGridGate(transformer, transformer.getNodeA(), transformer.getNodeB());
-      case C:
-        return new SubGridGate(transformer, transformer.getNodeA(), transformer.getNodeC());
-      default:
-        throw new IllegalArgumentException(
-            "Only port "
-                + ConnectorPort.B
-                + " or "
-                + ConnectorPort.C
-                + " can be "
-                + "chosen as inferior port.");
-    }
-  }
-
-  private final TransformerInput link;
-  private final NodeInput superiorNode;
-  private final NodeInput inferiorNode;
-
-  /**
-   * Create a {@link SubGridGate}
-   *
-   * @param link Model, that physically represents the gate
-   * @param superiorNode Upstream node of the gate
-   * @param inferiorNode Downstream node of the gate
-   */
-  public SubGridGate(TransformerInput link, NodeInput superiorNode, NodeInput inferiorNode) {
-    this.link = link;
-    this.superiorNode = superiorNode;
-    this.inferiorNode = inferiorNode;
+    return switch (inferiorPort) {
+      case B -> new SubGridGate(transformer, transformer.getNodeA(), transformer.getNodeB());
+      case C -> new SubGridGate(transformer, transformer.getNodeA(), transformer.getNodeC());
+      default -> throw new IllegalArgumentException(
+          "Only port "
+              + ConnectorPort.B
+              + " or "
+              + ConnectorPort.C
+              + " can be "
+              + "chosen as inferior port.");
+    };
   }
 
+  /** @deprecated since 3.0. Use {@link #link()} instead */
+  @Deprecated(since = "3.0")
   public TransformerInput getLink() {
     return link;
   }
 
+  /** @deprecated since 3.0. Use {@link #superiorNode()} instead */
+  @Deprecated(since = "3.0")
   public NodeInput getSuperiorNode() {
     return superiorNode;
   }
 
+  /** @deprecated since 3.0. Use {@link #inferiorNode()} instead */
+  @Deprecated(since = "3.0")
   public NodeInput getInferiorNode() {
     return inferiorNode;
   }
@@ -93,21 +79,6 @@ public int getInferiorSubGrid() {
     return inferiorNode.getSubnet();
   }
 
-  @Override
-  public boolean equals(Object o) {
-    if (this == o) return true;
-    if (o == null || getClass() != o.getClass()) return false;
-    SubGridGate that = (SubGridGate) o;
-    return link.equals(that.link)
-        && superiorNode.equals(that.superiorNode)
-        && inferiorNode.equals(that.inferiorNode);
-  }
-
-  @Override
-  public int hashCode() {
-    return Objects.hash(link, superiorNode, inferiorNode);
-  }
-
   @Override
   public String toString() {
     return "SubGridTopolgyEdge{"

src/main/java/edu/ie3/datamodel/io/connectors/CsvFileConnector.java

@@ -111,7 +111,7 @@ private BufferedCsvWriter initWriter(String baseDirectory, CsvFileDefinition fil
     /* Join the full DIRECTORY path (excluding file name) */
     String baseDirectoryHarmonized = IoUtil.harmonizeFileSeparator(baseDirectory);
     String fullDirectoryPath =
-        FilenameUtils.concat(baseDirectoryHarmonized, fileDefinition.getDirectoryPath());
+        FilenameUtils.concat(baseDirectoryHarmonized, fileDefinition.directoryPath());
     String fullPath = FilenameUtils.concat(baseDirectoryHarmonized, fileDefinition.getFilePath());
 
     /* Create missing directories */
@@ -125,13 +125,13 @@ private BufferedCsvWriter initWriter(String baseDirectory, CsvFileDefinition fil
     boolean append = pathFile.exists();
     BufferedCsvWriter writer =
         new BufferedCsvWriter(
-            fullPath, fileDefinition.getHeadLineElements(), fileDefinition.getCsvSep(), append);
+            fullPath, fileDefinition.headLineElements(), fileDefinition.csvSep(), append);
     if (!append) {
       writer.writeFileHeader();
     } else {
       log.warn(
           "File '{}' already exist. Will append new content WITHOUT new header! Full path: {}",
-          fileDefinition.getFileName(),
+          fileDefinition.fileName(),
           pathFile.getAbsolutePath());
     }
     return writer;
@@ -161,7 +161,7 @@ public synchronized void closeTimeSeriesWriter(UUID uuid) throws IOException {
    * @param <C> Type of class
    * @throws IOException If closing of writer fails.
    */
-  public synchronized <C extends Class<? extends UniqueEntity>> void closeEntityWriter(C clz)
+  public synchronized <C extends UniqueEntity> void closeEntityWriter(Class<C> clz)
       throws IOException {
     Optional<BufferedCsvWriter> maybeWriter = Optional.ofNullable(entityWriters.get(clz));
     if (maybeWriter.isPresent()) {

src/main/java/edu/ie3/datamodel/io/csv/BufferedCsvWriter.java

@@ -63,8 +63,8 @@ public BufferedCsvWriter(String baseFolder, CsvFileDefinition fileDefinition, bo
       throws IOException {
     this(
         baseFolder + File.separator + fileDefinition.getFilePath(),
-        fileDefinition.getHeadLineElements(),
-        fileDefinition.getCsvSep(),
+        fileDefinition.headLineElements(),
+        fileDefinition.csvSep(),
         append);
   }
 

src/main/java/edu/ie3/datamodel/io/csv/CsvFileDefinition.java

@@ -14,19 +14,15 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class CsvFileDefinition {
+public record CsvFileDefinition(
+    String fileName, String directoryPath, String[] headLineElements, String csvSep) {
   private static final Logger logger = LoggerFactory.getLogger(CsvFileDefinition.class);
 
   private static final Pattern FILE_NAME_PATTERN =
       Pattern.compile(
           "^(?<fileName>[^\\\\/\\s.]{0,255})(?:\\.(?<extension>[a-zA-Z0-9]{0,10}(?:\\.[a-zA-Z0-9]{0,10})?))?$");
 
-  protected static final String FILE_EXTENSION = "csv";
-
-  private final String directoryPath;
-  private final String fileName;
-  private final String[] headLineElements;
-  private final String csvSep;
+  private static final String FILE_EXTENSION = "csv";
 
   public CsvFileDefinition(
       String fileName, String directoryPath, String[] headLineElements, String csvSep) {
@@ -60,11 +56,17 @@ public CsvFileDefinition(
     this.csvSep = csvSep;
   }
 
+  /** @deprecated since 3.0. Use {@link #directoryPath()} instead */
+  @Deprecated(since = "3.0")
   public String getDirectoryPath() {
     return directoryPath;
   }
 
-  /** @return The file name including extension */
+  /**
+   * @return The file name including extension
+   * @deprecated since 3.0. Use {@link #fileName()} instead
+   */
+  @Deprecated(since = "3.0")
   public String getFileName() {
     return fileName;
   }
@@ -77,19 +79,24 @@ public String getFilePath() {
     return !directoryPath.isEmpty() ? FilenameUtils.concat(directoryPath, fileName) : fileName;
   }
 
+  /** @deprecated since 3.0. Use {@link #headLineElements()} instead */
+  @Deprecated(since = "3.0")
   public String[] getHeadLineElements() {
     return headLineElements;
   }
 
+  /** @deprecated since 3.0. Use {@link #csvSep()} instead */
+  @Deprecated(since = "3.0")
   public String getCsvSep() {
     return csvSep;
   }
 
   @Override
   public boolean equals(Object o) {
+    // equals implementation is required here because
+    // records' equals method and array fields don't play together nicely
     if (this == o) return true;
-    if (!(o instanceof CsvFileDefinition)) return false;
-    CsvFileDefinition that = (CsvFileDefinition) o;
+    if (!(o instanceof CsvFileDefinition that)) return false;
     return directoryPath.equals(that.directoryPath)
         && fileName.equals(that.fileName)
         && Arrays.equals(headLineElements, that.headLineElements)

src/main/java/edu/ie3/datamodel/io/csv/timeseries/IndividualTimeSeriesMetaInformation.java

@@ -45,9 +45,8 @@ public ColumnScheme getColumnScheme() {
   @Override
   public boolean equals(Object o) {
     if (this == o) return true;
-    if (!(o instanceof IndividualTimeSeriesMetaInformation)) return false;
+    if (!(o instanceof IndividualTimeSeriesMetaInformation that)) return false;
     if (!super.equals(o)) return false;
-    IndividualTimeSeriesMetaInformation that = (IndividualTimeSeriesMetaInformation) o;
     return columnScheme == that.columnScheme;
   }
 

src/main/java/edu/ie3/datamodel/io/csv/timeseries/LoadProfileTimeSeriesMetaInformation.java

@@ -37,9 +37,8 @@ public String getProfile() {
   @Override
   public boolean equals(Object o) {
     if (this == o) return true;
-    if (!(o instanceof LoadProfileTimeSeriesMetaInformation)) return false;
+    if (!(o instanceof LoadProfileTimeSeriesMetaInformation that)) return false;
     if (!super.equals(o)) return false;
-    LoadProfileTimeSeriesMetaInformation that = (LoadProfileTimeSeriesMetaInformation) o;
     return profile.equals(that.profile);
   }
 

src/main/java/edu/ie3/datamodel/io/extractor/Extractor.java

@@ -34,7 +34,7 @@ public static Set<InputEntity> extractElements(NestedEntity nestedEntity)
       throws ExtractorException {
     CopyOnWriteArrayList<InputEntity> resultingList = new CopyOnWriteArrayList<>();
     if (nestedEntity instanceof HasNodes nestedHasNode) {
-      resultingList.addAll((nestedHasNode).allNodes());
+      resultingList.addAll(nestedHasNode.allNodes());
     }
     if (nestedEntity instanceof Operable nestedOperable) {
       extractOperator(nestedOperable).ifPresent(resultingList::add);
@@ -43,13 +43,13 @@ public static Set<InputEntity> extractElements(NestedEntity nestedEntity)
       resultingList.add(extractType(nestedHasType));
     }
     if (nestedEntity instanceof HasThermalBus nestedHasThermalBus) {
-      resultingList.add((nestedHasThermalBus).getThermalBus());
+      resultingList.add(nestedHasThermalBus.getThermalBus());
     }
     if (nestedEntity instanceof HasThermalStorage nestedHasThermalStorage) {
-      resultingList.add((nestedHasThermalStorage).getThermalStorage());
+      resultingList.add(nestedHasThermalStorage.getThermalStorage());
     }
     if (nestedEntity instanceof HasLine nestedHasLine) {
-      resultingList.add((nestedHasLine).getLine());
+      resultingList.add(nestedHasLine.getLine());
     }
 
     if (resultingList.contains(null)) {

src/main/java/edu/ie3/datamodel/io/factory/FactoryData.java

@@ -140,8 +140,7 @@ field, getField(field)),
   @Override
   public boolean equals(Object o) {
     if (this == o) return true;
-    if (!(o instanceof FactoryData)) return false;
-    FactoryData that = (FactoryData) o;
+    if (!(o instanceof FactoryData that)) return false;
     return fieldsToAttributes.equals(that.fieldsToAttributes)
         && targetClass.equals(that.targetClass);
   }

src/main/java/edu/ie3/datamodel/io/factory/input/ThermalUnitInputEntityData.java

@@ -38,9 +38,8 @@ public ThermalBusInput getBusInput() {
   @Override
   public boolean equals(Object o) {
     if (this == o) return true;
-    if (!(o instanceof ThermalUnitInputEntityData)) return false;
+    if (!(o instanceof ThermalUnitInputEntityData that)) return false;
     if (!super.equals(o)) return false;
-    ThermalUnitInputEntityData that = (ThermalUnitInputEntityData) o;
     return busInput.equals(that.busInput);
   }