From 965311ff4e2e38ed7d74bf1999eb61ceb3200e64 Mon Sep 17 00:00:00 2001
From: Billy Olsen <billy.olsen@gmail.com>
Date: Fri, 4 Jun 2021 20:34:01 -0700
Subject: [PATCH] FlightConfigurableParameterSet Predictable Orders

Change to use a LinkedHashMap instead of a HashMap in the
FlightConfigurableParameterSet. This ensures that the ordering is
consistent with the way the user added the flight configurations.

This will also be true for when reading the flight configurations from
a file. Additionally, to preserve the ordering, do not sort the
FlightConfigurationIds in getIds().

Fixes #845

Signed-off-by: Billy Olsen <billy.olsen@gmail.com>
---
 .../FlightConfigurableParameterSet.java       | 10 ++----
 .../rocketcomponent/ParameterSetTest.java     | 35 ++++++++++++++++---
 2 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/core/src/net/sf/openrocket/rocketcomponent/FlightConfigurableParameterSet.java b/core/src/net/sf/openrocket/rocketcomponent/FlightConfigurableParameterSet.java
index cc4ef365b..91dbf693d 100644
--- a/core/src/net/sf/openrocket/rocketcomponent/FlightConfigurableParameterSet.java
+++ b/core/src/net/sf/openrocket/rocketcomponent/FlightConfigurableParameterSet.java
@@ -1,8 +1,7 @@
 package net.sf.openrocket.rocketcomponent;
 
-import java.util.Collections;
-import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map.Entry;
 
@@ -21,7 +20,7 @@ import net.sf.openrocket.util.Utils;
 public class FlightConfigurableParameterSet<E extends FlightConfigurableParameter<E>> implements Iterable<E> {
 	
 	//private static final Logger log = LoggerFactory.getLogger(ParameterSet.class);
-	protected final HashMap<FlightConfigurationId, E> map = new HashMap<FlightConfigurationId, E>();
+	protected final LinkedHashMap<FlightConfigurationId, E> map = new LinkedHashMap<FlightConfigurationId, E>();
 
 	/**
 	 * Construct a FlightConfiguration that has no overrides.
@@ -155,11 +154,6 @@ public class FlightConfigurableParameterSet<E extends FlightConfigurableParamete
 
         toReturn.addAll( this.map.keySet() );
         toReturn.remove( FlightConfigurationId.DEFAULT_VALUE_FCID );
-        // Java 1.8:
-        //toReturn.sort( null );
-
-        // Java 1.7:
-        Collections.sort(toReturn);
 
         return toReturn;
 	}
diff --git a/core/test/net/sf/openrocket/rocketcomponent/ParameterSetTest.java b/core/test/net/sf/openrocket/rocketcomponent/ParameterSetTest.java
index 2039ebd28..e6fbf9b1d 100644
--- a/core/test/net/sf/openrocket/rocketcomponent/ParameterSetTest.java
+++ b/core/test/net/sf/openrocket/rocketcomponent/ParameterSetTest.java
@@ -7,9 +7,10 @@ package net.sf.openrocket.rocketcomponent;
 
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
 
-import java.util.Collections;
+import java.util.List;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -174,7 +175,6 @@ public class ParameterSetTest extends BaseTestCase {
 		refList.add(fcid2);
 		refList.add(fcid3);
 		refList.add(fcid4);
-	    Collections.sort(refList); 		// Java 1.7:
 	    
 		//assertThat
 	    assertThat("retrieve-by-index broken!\n"+testSet.toDebug(), testSet.get(0), equalTo( testSet.get( refList.get(0))));
@@ -241,6 +241,33 @@ public class ParameterSetTest extends BaseTestCase {
 		assertThat("set stores default value correctly: ", testSet.get(fcid3), not( testSet.getDefault() )); 
 	}
 	
-	
-	
+	/**
+	 * Confirms the ordering of the flights are as inserted.
+	 */
+	@Test
+	public void testOrdering() {
+		TestParameter tp1 = new TestParameter();
+		FlightConfigurationId fcid1 = new FlightConfigurationId();
+		testSet.set(fcid1, tp1);
+		
+		TestParameter tp2 = new TestParameter();
+		FlightConfigurationId fcid2 = new FlightConfigurationId();
+		testSet.set(fcid2, tp2);
+		
+		TestParameter tp3 = new TestParameter();
+		FlightConfigurationId fcid3 = new FlightConfigurationId();
+		testSet.set(fcid3, tp3);
+		
+		TestParameter tp4 = new TestParameter();
+		FlightConfigurationId fcid4 = new FlightConfigurationId();
+		testSet.set(fcid4, tp4);
+		
+		List<FlightConfigurationId> refList = new ArrayList<FlightConfigurationId>();
+		refList.add(fcid1);
+		refList.add(fcid2);
+		refList.add(fcid3);
+		refList.add(fcid4);
+		
+		assertEquals(refList, testSet.getIds());
+	}
 }