Merge pull request #133 from cicirello/fix

Misc improvements based on SpotBugs analysis

CHANGELOG.md

@@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [Unreleased] - 2023-08-05
+## [Unreleased] - 2023-08-06
 
 ### Added
 
@@ -15,6 +15,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Removed
 
 ### Fixed
+* LogRecord.toString() changed to use platform-specific line separators.
+* LogRecord.equals(Object): correctly handle null and other types.
+* RecordList.equals(Object): correctly handle null and other types.
+* Bin.contentsToString(): StringBuilder instead of iterated concat.
+* SessionLog.moveCountToString(): StringBuilder instead of iterated concat.
+* SessionLogFormatter: fixed potential resource leak.
+* SessionLog: implemented readObject to properly initialize transient fields during deserialization.
+* Set most classes to package access.
 
 ### CI/CD
 * Integrated SpotBugs into build process.

spotbugs-exclude.xml

@@ -4,7 +4,7 @@
     <Bug pattern="PREDICTABLE_RANDOM" />
   </Match>
 
-  <!-- False positive -->
+  <!-- False positive: using JFileChooser with filter to select file, SpotBugs complaining where extension is added if missing -->
   <Match>
     <Bug pattern="PATH_TRAVERSAL_IN" />
 	<Class name="org.cicirello.ibp.MenuBar" />

src/main/java/org/cicirello/ibp/About.java

@@ -1,6 +1,6 @@
 /*
  * Interactive Bin Packing.
- * Copyright (C) 2008, 2010, 2020-2021  Vincent A. Cicirello
+ * Copyright (C) 2008, 2010, 2020-2023 Vincent A. Cicirello
  *
  * This file is part of Interactive Bin Packing.
  *
@@ -38,7 +38,7 @@
  * @author <a href=https://www.cicirello.org/ target=_top>Vincent A. Cicirello</a>, <a
  *     href=https://www.cicirello.org/ target=_top>https://www.cicirello.org/</a>
  */
-public class About extends JDialog {
+final class About extends JDialog {
 
   /**
    * Constructs the About dialog.

src/main/java/org/cicirello/ibp/ApplicationState.java

@@ -1,6 +1,6 @@
 /*
  * Interactive Bin Packing.
- * Copyright (C) 2008, 2010, 2020-2021  Vincent A. Cicirello
+ * Copyright (C) 2008, 2010, 2020-2023 Vincent A. Cicirello
  *
  * This file is part of Interactive Bin Packing.
  *
@@ -34,7 +34,7 @@
  * @author <a href=https://www.cicirello.org/ target=_top>Vincent A. Cicirello</a>, <a
  *     href=https://www.cicirello.org/ target=_top>https://www.cicirello.org/</a>
  */
-public class ApplicationState {
+final class ApplicationState {
 
   /** Practice mode where user is free to move items around as they see fit. */
   public static final int MODE_PRACTICE = 0;

src/main/java/org/cicirello/ibp/Bin.java

@@ -1,6 +1,6 @@
 /*
  * Interactive Bin Packing.
- * Copyright (C) 2008, 2010, 2020-2022 Vincent A. Cicirello
+ * Copyright (C) 2008, 2010, 2020-2023 Vincent A. Cicirello
  *
  * This file is part of Interactive Bin Packing.
  *
@@ -31,7 +31,7 @@
  * @author <a href=https://www.cicirello.org/ target=_top>Vincent A. Cicirello</a>, <a
  *     href=https://www.cicirello.org/ target=_top>https://www.cicirello.org/</a>
  */
-public class Bin {
+class Bin {
 
   private ArrayList<Item> contents;
   private int capacity;
@@ -214,17 +214,18 @@ public void removeAll() {
    * @return a String listing all of the objects in the bin
    */
   public String contentsToString() {
-    String result = "";
+    StringBuilder result = new StringBuilder();
     int i = 0;
     for (; i < contents.size() - 1; i++) {
-      result += contents.get(i) + ", ";
+      result.append(contents.get(i));
+      result.append(", ");
     }
     if (contents.size() > 0) {
-      result += contents.get(i);
+      result.append(contents.get(i));
     } else {
-      result = "empty";
+      result.append("empty");
     }
-    return result;
+    return result.toString();
   }
 
   /**

src/main/java/org/cicirello/ibp/BottomPanel.java

@@ -1,6 +1,6 @@
 /*
  * Interactive Bin Packing.
- * Copyright (C) 2008, 2010, 2020-2021  Vincent A. Cicirello
+ * Copyright (C) 2008, 2010, 2020-2023 Vincent A. Cicirello
  *
  * This file is part of Interactive Bin Packing.
  *
@@ -40,13 +40,13 @@
  * @author <a href=https://www.cicirello.org/ target=_top>Vincent A. Cicirello</a>, <a
  *     href=https://www.cicirello.org/ target=_top>https://www.cicirello.org/</a>
  */
-public class BottomPanel extends JPanel {
+class BottomPanel extends JPanel {
 
   /** Application. */
-  private InteractiveBinPacking f;
+  private final InteractiveBinPacking f;
 
   /** Maintains application state. */
-  private ApplicationState state;
+  private final ApplicationState state;
 
   /** Combo box of destinations. */
   private JComboBox<Bin> destinations;
@@ -55,7 +55,7 @@ public class BottomPanel extends JPanel {
   private JComboBox<Item> itemList;
 
   /** Called when item moved. */
-  private CallBack onMove;
+  private final CallBack onMove;
 
   /**
    * Constructs the panel.

src/main/java/org/cicirello/ibp/CallBack.java

@@ -1,6 +1,6 @@
 /*
  * Interactive Bin Packing.
- * Copyright (C) 2008, 2010, 2020-2021  Vincent A. Cicirello
+ * Copyright (C) 2008, 2010, 2020-2023 Vincent A. Cicirello
  *
  * This file is part of Interactive Bin Packing.
  *
@@ -27,7 +27,7 @@
  * @author <a href=https://www.cicirello.org/ target=_top>Vincent A. Cicirello</a>, <a
  *     href=https://www.cicirello.org/ target=_top>https://www.cicirello.org/</a>
  */
-public interface CallBack {
+interface CallBack {
 
   /** This is the callback method. */
   void call();

src/main/java/org/cicirello/ibp/CenterPanel.java

@@ -1,6 +1,6 @@
 /*
  * Interactive Bin Packing.
- * Copyright (C) 2008, 2010, 2020-2021  Vincent A. Cicirello
+ * Copyright (C) 2008, 2010, 2020-2023 Vincent A. Cicirello
  *
  * This file is part of Interactive Bin Packing.
  *
@@ -35,13 +35,13 @@
  * @author <a href=https://www.cicirello.org/ target=_top>Vincent A. Cicirello</a>, <a
  *     href=https://www.cicirello.org/ target=_top>https://www.cicirello.org/</a>
  */
-public class CenterPanel extends JPanel {
+final class CenterPanel extends JPanel {
 
   /** Contents of the bins. */
-  private ArrayList<JTextField> binContents;
+  private final ArrayList<JTextField> binContents;
 
   /** Maintains application state. */
-  private ApplicationState state;
+  private final ApplicationState state;
 
   /**
    * Constructs the panel.

src/main/java/org/cicirello/ibp/Floor.java

@@ -1,6 +1,6 @@
 /*
  * Interactive Bin Packing.
- * Copyright (C) 2008, 2010, 2020-2022 Vincent A. Cicirello
+ * Copyright (C) 2008, 2010, 2020-2023 Vincent A. Cicirello
  *
  * This file is part of Interactive Bin Packing.
  *
@@ -30,7 +30,7 @@
  * @author <a href=https://www.cicirello.org/ target=_top>Vincent A. Cicirello</a>, <a
  *     href=https://www.cicirello.org/ target=_top>https://www.cicirello.org/</a>
  */
-public class Floor extends Bin {
+final class Floor extends Bin {
 
   /**
    * Generates a random instance of the bin packing problem. Through the use of a seed for the

src/main/java/org/cicirello/ibp/Help.java

@@ -1,6 +1,6 @@
 /*
  * Interactive Bin Packing.
- * Copyright (C) 2008, 2010, 2020-2021  Vincent A. Cicirello
+ * Copyright (C) 2008, 2010, 2020-2023 Vincent A. Cicirello
  *
  * This file is part of Interactive Bin Packing.
  *
@@ -29,7 +29,7 @@
  * @author <a href=https://www.cicirello.org/ target=_top>Vincent A. Cicirello</a>, <a
  *     href=https://www.cicirello.org/ target=_top>https://www.cicirello.org/</a>
  */
-public class Help extends InfoDialog {
+final class Help extends InfoDialog {
 
   /**
    * Constructs the Help dialog.

src/main/java/org/cicirello/ibp/InfoDialog.java

@@ -1,6 +1,6 @@
 /*
  * Interactive Bin Packing.
- * Copyright (C) 2008, 2010, 2020-2021  Vincent A. Cicirello
+ * Copyright (C) 2008, 2010, 2020-2023 Vincent A. Cicirello
  *
  * This file is part of Interactive Bin Packing.
  *
@@ -42,7 +42,7 @@
  * @author <a href=https://www.cicirello.org/ target=_top>Vincent A. Cicirello</a>, <a
  *     href=https://www.cicirello.org/ target=_top>https://www.cicirello.org/</a>
  */
-public class InfoDialog extends JDialog {
+class InfoDialog extends JDialog {
 
   /**
    * Constructs an InfoDialog.

src/main/java/org/cicirello/ibp/InteractiveBinPacking.java

@@ -1,6 +1,6 @@
 /*
  * Interactive Bin Packing.
- * Copyright (C) 2008, 2010, 2020-2022  Vincent A. Cicirello
+ * Copyright (C) 2008, 2010, 2020-2023 Vincent A. Cicirello
  *
  * This file is part of Interactive Bin Packing.
  *

src/main/java/org/cicirello/ibp/Item.java

@@ -1,6 +1,6 @@
 /*
  * Interactive Bin Packing.
- * Copyright (C) 2008, 2010, 2020-2021  Vincent A. Cicirello
+ * Copyright (C) 2008, 2010, 2020-2023 Vincent A. Cicirello
  *
  * This file is part of Interactive Bin Packing.
  *
@@ -28,10 +28,10 @@
  * @author <a href=https://www.cicirello.org/ target=_top>Vincent A. Cicirello</a>, <a
  *     href=https://www.cicirello.org/ target=_top>https://www.cicirello.org/</a>
  */
-public class Item {
+final class Item {
 
-  private String name;
-  private int size;
+  private final String name;
+  private final int size;
 
   /**
    * Construct an item.

src/main/java/org/cicirello/ibp/LogRecord.java

@@ -57,13 +57,10 @@ long getTimestamp() {
   }
 
   @Override
-  @SuppressWarnings("EqualsUnsafeCast")
   public boolean equals(Object other) {
-    /* // UNNECESSARY CHECK... access control prevents these cases
     if (other == null || !(other instanceof LogRecord)) {
-    	return false;
+      return false;
     }
-    */
     LogRecord o = (LogRecord) other;
     return type.equals(o.type) && data.equals(o.data) && timestamp == o.timestamp;
   }
@@ -80,7 +77,7 @@ public int hashCode() {
   public String toString() {
     // Used in generating session log file contents.
     return String.format(
-        "<action>\n<type>%s</type>\n<data>%s</data>\n<timestamp>%d</timestamp>\n</action>\n",
+        "<action>%n<type>%s</type>%n<data>%s</data>%n<timestamp>%d</timestamp>%n</action>%n",
         type, data, timestamp);
   }
 }

src/main/java/org/cicirello/ibp/MenuBar.java

@@ -46,13 +46,13 @@
  * @author <a href=https://www.cicirello.org/ target=_top>Vincent A. Cicirello</a>, <a
  *     href=https://www.cicirello.org/ target=_top>https://www.cicirello.org/</a>
  */
-public class MenuBar extends JMenuBar {
+class MenuBar extends JMenuBar {
 
   /** Application. */
   private InteractiveBinPacking f;
 
   /** Maintains application state. */
-  private ApplicationState state;
+  private final ApplicationState state;
 
   /** Menu item for sorting. */
   private JMenuItem sortItem;
@@ -67,7 +67,7 @@ public class MenuBar extends JMenuBar {
   private Help help;
 
   /** The file chooser. */
-  private JFileChooser chooser;
+  private final JFileChooser chooser;
 
   /**
    * Constructs the menu bar.

src/main/java/org/cicirello/ibp/RecordList.java

@@ -28,13 +28,10 @@ final class RecordList extends ArrayList<LogRecord> {
   private static final long serialVersionUID = 1L;
 
   @Override
-  @SuppressWarnings("EqualsUnsafeCast")
   public boolean equals(Object other) {
-    /* // UNNECESSARY CHECK... access control prevents these cases
     if (other == null || !(other instanceof RecordList)) {
-    	return false;
+      return false;
     }
-    */
     RecordList r = (RecordList) other;
     if (size() != r.size()) {
       return false;

src/main/java/org/cicirello/ibp/SessionLog.java

@@ -21,6 +21,8 @@
 
 package org.cicirello.ibp;
 
+import java.io.IOException;
+import java.io.ObjectInputStream;
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -37,7 +39,7 @@
  * @author <a href=https://www.cicirello.org/ target=_top>Vincent A. Cicirello</a>, <a
  *     href=https://www.cicirello.org/ target=_top>https://www.cicirello.org/</a>
  */
-public final class SessionLog implements Serializable {
+final class SessionLog implements Serializable {
 
   private static final long serialVersionUID = 1L;
 
@@ -47,7 +49,7 @@ public final class SessionLog implements Serializable {
   /** Counts of successful moves in each mode. */
   private final int[] successfulMoves;
 
-  /** Counts of failed mvoes in each mode. */
+  /** Counts of failed moves in each mode. */
   private final int[] failedMoves;
 
   private transient int currentMode;
@@ -67,6 +69,21 @@ public SessionLog() {
     currentBinSequence = new ArrayList<Integer>();
   }
 
+  /**
+   * readObject method for deserialization.
+   *
+   * @param in the ObjectInputStream
+   * @throws IOException if an I/O error occurs
+   * @throws ClassNotFoundException if the class of a serialized object could not be found
+   */
+  private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
+    in.defaultReadObject();
+    currentMode = 0;
+    currentInstance = "Default";
+    currentItemSequence = new ArrayList<Item>();
+    currentBinSequence = new ArrayList<Integer>();
+  }
+
   /**
    * Adds an entry to the session log. Also does any entry type specific operations.
    *
@@ -182,12 +199,12 @@ public String toString() {
   private String moveCountToString(boolean successful) {
     int[] counts = successful ? successfulMoves : failedMoves;
     String template = successful ? "<successful>%s</successful>\n" : "<failed>%s</failed>\n";
-    String strCounts = "";
+    StringBuilder strCounts = new StringBuilder();
     String oneCount = "%d ";
     for (int c : counts) {
-      strCounts += String.format(oneCount, c);
+      strCounts.append(String.format(oneCount, c));
     }
-    return String.format(template, strCounts.strip());
+    return String.format(template, strCounts.toString().strip());
   }
 
   static SessionLog createSessionLogFromFile(Readable file) {