Improve equals contract in People Plugin

simulation/src/main/java/gov/hhs/aspr/ms/gcm/simulation/plugins/people/datamanagers/PeoplePluginData.java

@@ -3,6 +3,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
 
 import gov.hhs.aspr.ms.gcm.simulation.nucleus.PluginData;
 import gov.hhs.aspr.ms.gcm.simulation.nucleus.PluginDataBuilder;
@@ -44,44 +45,34 @@ private Data(Data data) {
 			this.assignmentTime = data.assignmentTime;
 		}
 
+		/**
+    	 * Standard implementation consistent with the {@link #equals(Object)} method
+    	 */
 		@Override
 		public int hashCode() {
-			/*
-			 * See notes in equals()
-			 */
-			final int prime = 31;
-			int result = 1;
-			long temp = Double.doubleToLongBits(assignmentTime);
-			result = prime * result + (int) (temp ^ (temp >>> 32));
-			result = prime * result + personCount;
-			result = prime * result + personRanges.hashCode();
-			return result;
+			return Objects.hash(personCount, personRanges, assignmentTime);
 		}
 
+		/**
+    	 * Two {@link Data} instances are equal if and only if
+    	 * their inputs are equal. Person ranges are sorted and
+		 * joined during the build process, leaving the person
+		 * ranges unambiguously ordered.
+    	 */
 		@Override
 		public boolean equals(Object obj) {
-			/*
-			 * This boilerplate implementation works since the person ranges are sorted and
-			 * joined during the build process, leaving the person ranges unambiguously
-			 * ordered.
-			 */
 			if (this == obj) {
 				return true;
 			}
-			if (!(obj instanceof Data)) {
-				return false;
-			}
-			Data other = (Data) obj;
-			if (Double.doubleToLongBits(assignmentTime) != Double.doubleToLongBits(other.assignmentTime)) {
-				return false;
-			}
-			if (personCount != other.personCount) {
+			if (obj == null) {
 				return false;
 			}
-			if (!personRanges.equals(other.personRanges)) {
+			if (getClass() != obj.getClass()) {
 				return false;
 			}
-			return true;
+			Data other = (Data) obj;
+			return personCount == other.personCount && Objects.equals(personRanges, other.personRanges)
+					&& Double.doubleToLongBits(assignmentTime) == Double.doubleToLongBits(other.assignmentTime);
 		}
 
 		@Override
@@ -346,27 +337,31 @@ public static boolean checkVersionSupported(String version) {
 		return StandardVersioning.checkVersionSupported(version);
 	}
 
+	/**
+	 * Standard implementation consistent with the {@link #equals(Object)} method
+	 */
 	@Override
 	public int hashCode() {
-		final int prime = 31;
-		int result = 1;
-		result = prime * result + data.hashCode();
-		return result;
+		return Objects.hash(data);
 	}
 
+	/**
+	 * Two {@link PeoplePluginData} instances are equal if and only if
+	 * their inputs are equal.
+	 */
 	@Override
 	public boolean equals(Object obj) {
 		if (this == obj) {
 			return true;
 		}
-		if (!(obj instanceof PeoplePluginData)) {
+		if (obj == null) {
 			return false;
 		}
-		PeoplePluginData other = (PeoplePluginData) obj;
-		if (!data.equals(other.data)) {
+		if (getClass() != obj.getClass()) {
 			return false;
 		}
-		return true;
+		PeoplePluginData other = (PeoplePluginData) obj;
+		return Objects.equals(data, other.data);
 	}
 
 	/**

simulation/src/main/java/gov/hhs/aspr/ms/gcm/simulation/plugins/people/support/PersonId.java

@@ -1,5 +1,7 @@
 package gov.hhs.aspr.ms.gcm.simulation.plugins.people.support;
 
+import java.util.Objects;
+
 import gov.hhs.aspr.ms.util.errors.ContractException;
 import net.jcip.annotations.Immutable;
 
@@ -32,24 +34,31 @@ public int compareTo(PersonId personId) {
 		return Integer.compare(id, personId.id);
 	}
 
+	/**
+	 * Standard implementation consistent with the {@link #equals(Object)} method
+	 */
 	@Override
 	public int hashCode() {
-		return id;
+		return Objects.hash(id);
 	}
 
+	/**
+	 * Two {@link PersonId} instances are equal if and only if
+	 * their inputs are equal.
+	 */
 	@Override
 	public boolean equals(Object obj) {
 		if (this == obj) {
 			return true;
 		}
-		if (!(obj instanceof PersonId)) {
+		if (obj == null) {
 			return false;
 		}
-		PersonId other = (PersonId) obj;
-		if (id != other.id) {
+		if (getClass() != obj.getClass()) {
 			return false;
 		}
-		return true;
+		PersonId other = (PersonId) obj;
+		return id == other.id;
 	}
 
 	@Override

simulation/src/main/java/gov/hhs/aspr/ms/gcm/simulation/plugins/people/support/PersonRange.java

@@ -1,5 +1,7 @@
 package gov.hhs.aspr.ms.gcm.simulation.plugins.people.support;
 
+import java.util.Objects;
+
 import gov.hhs.aspr.ms.util.errors.ContractException;
 import net.jcip.annotations.Immutable;
 
@@ -66,37 +68,30 @@ public int compareTo(PersonRange personRange) {
 	}
 
 	/**
-	 * Boiler plate implementation of hash code
+	 * Standard implementation consistent with the {@link #equals(Object)} method
 	 */
 	@Override
 	public int hashCode() {
-		final int prime = 31;
-		int result = 1;
-		result = prime * result + highPersonId;
-		result = prime * result + lowPersonId;
-		return result;
+		return Objects.hash(lowPersonId, highPersonId);
 	}
 
 	/**
-	 * Two person ranges are equal if and only if they have the same upper and lower
-	 * bounds.
+	 * Two {@link PersonRange} instances are equal if and only if
+	 * their inputs are equal.
 	 */
 	@Override
 	public boolean equals(Object obj) {
 		if (this == obj) {
 			return true;
 		}
-		if (!(obj instanceof PersonRange)) {
-			return false;
-		}
-		PersonRange other = (PersonRange) obj;
-		if (highPersonId != other.highPersonId) {
+		if (obj == null) {
 			return false;
 		}
-		if (lowPersonId != other.lowPersonId) {
+		if (getClass() != obj.getClass()) {
 			return false;
 		}
-		return true;
+		PersonRange other = (PersonRange) obj;
+		return lowPersonId == other.lowPersonId && highPersonId == other.highPersonId;
 	}
 
 	/**

simulation/src/test/java/gov/hhs/aspr/ms/gcm/simulation/plugins/people/support/AT_PersonId.java

@@ -1,15 +1,20 @@
 package gov.hhs.aspr.ms.gcm.simulation.plugins.people.support;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.util.LinkedHashSet;
+import java.util.Set;
+
+import org.apache.commons.math3.random.RandomGenerator;
 import org.junit.jupiter.api.Test;
 
 import gov.hhs.aspr.ms.util.annotations.UnitTestConstructor;
 import gov.hhs.aspr.ms.util.annotations.UnitTestMethod;
 import gov.hhs.aspr.ms.util.errors.ContractException;
+import gov.hhs.aspr.ms.util.random.RandomGeneratorProvider;
 
 
 public final class AT_PersonId {
@@ -51,17 +56,45 @@ public void testCompareTo() {
 	@Test
 	@UnitTestMethod(target = PersonId.class,name = "equals", args = { Object.class })
 	public void testEquals() {
-		for (int i = 0; i < 10; i++) {
-			PersonId personA = new PersonId(i);
-			for (int j = 0; j < 10; j++) {
-				PersonId personB = new PersonId(j);				
-				if (i == j) {
-					assertEquals(personA,personB);
-				} else {
-					assertNotEquals(personA,personB);
-				}
+        RandomGenerator randomGenerator = RandomGeneratorProvider.getRandomGenerator(8980821493937106496L);
+
+        // never equal to another type
+		for (int i = 0; i < 30; i++) {
+            PersonId personId = getRandomPersonId(randomGenerator.nextLong());
+            assertFalse(personId.equals(new Object()));
+		}
+
+		// never equal to null
+		for (int i = 0; i < 30; i++) {
+			PersonId personId = getRandomPersonId(randomGenerator.nextLong());
+			assertFalse(personId.equals(null));
+		}
+
+		// reflexive
+		for (int i = 0; i < 30; i++) {
+			PersonId personId = getRandomPersonId(randomGenerator.nextLong());
+			assertTrue(personId.equals(personId));
+		}
+
+		// symmetric, transitive, consistent
+		for (int i = 0; i < 30; i++) {
+			long seed = randomGenerator.nextLong();
+			PersonId personId1 = getRandomPersonId(seed);
+			PersonId personId2 = getRandomPersonId(seed);
+			assertFalse(personId1 == personId2);
+			for (int j = 0; j < 10; j++) {				
+				assertTrue(personId1.equals(personId2));
+				assertTrue(personId2.equals(personId1));
 			}
 		}
+
+		// different inputs yield unequal PersonIds
+		Set<PersonId> set = new LinkedHashSet<>();
+		for (int i = 0; i < 100; i++) {
+			PersonId personId = getRandomPersonId(randomGenerator.nextLong());
+			set.add(personId);
+		}
+		assertEquals(100, set.size());
 	}
 
 	@Test
@@ -76,10 +109,26 @@ public void testGetValue() {
 	@Test
 	@UnitTestMethod(target = PersonId.class,name = "hashCode", args = {})
 	public void testHashCode() {
-		for (int i = 0; i < 10; i++) {
-			PersonId person = new PersonId(i);
-			assertEquals(i, person.hashCode());
-		}
+        RandomGenerator randomGenerator = RandomGeneratorProvider.getRandomGenerator(1948930019926275913L);
+
+        // equal objects have equal hash codes
+        for (int i = 0; i < 30; i++) {
+            long seed = randomGenerator.nextLong();
+			PersonId personId1 = getRandomPersonId(seed);
+			PersonId personId2 = getRandomPersonId(seed);
+
+            assertEquals(personId1, personId2);
+            assertEquals(personId1.hashCode(), personId2.hashCode());
+        }
+
+        // hash codes are reasonably distributed
+        Set<Integer> hashCodes = new LinkedHashSet<>();
+        for (int i = 0; i < 100; i++) {
+            PersonId personId = getRandomPersonId(randomGenerator.nextLong());
+            hashCodes.add(personId.hashCode());
+        }
+
+        assertEquals(100, hashCodes.size());
 	}
 
 	@Test
@@ -91,4 +140,8 @@ public void testToString() {
 		}
 	}
 
+	private PersonId getRandomPersonId(long seed) {
+		RandomGenerator randomGenerator = RandomGeneratorProvider.getRandomGenerator(seed);
+		return new PersonId(randomGenerator.nextInt(Integer.MAX_VALUE));
+	}
 }

simulation/src/test/java/gov/hhs/aspr/ms/gcm/simulation/plugins/people/support/AT_PersonRange.java

@@ -89,7 +89,7 @@ public void testCompareTo() {
 	@Test
 	@UnitTestMethod(target = PersonRange.class, name = "hashCode", args = {})
 	public void testHashCode() {
-		RandomGenerator randomGenerator = RandomGeneratorProvider.getRandomGenerator(8720935725310593369L);
+		RandomGenerator randomGenerator = RandomGeneratorProvider.getRandomGenerator(8720835725310593369L);
 		// show that equal person ranges have equal hashcodes
 		for (int i = 0; i < 10; i++) {
 			long seed = randomGenerator.nextLong();			
@@ -105,7 +105,7 @@ public void testHashCode() {
 			PersonRange personRange = getRandomPersonRange(randomGenerator.nextLong());
 			hashCodes.add(personRange.hashCode());
 		}
-		assertTrue(hashCodes.size() >= 90);
+		assertEquals(100, hashCodes.size());
 	}
 
 	private PersonRange getRandomPersonRange(long seed) {
@@ -119,13 +119,13 @@ private PersonRange getRandomPersonRange(long seed) {
 	@UnitTestMethod(target = PersonRange.class, name = "equals", args = { Object.class })
 	public void testEquals() {
 		RandomGenerator randomGenerator = RandomGeneratorProvider.getRandomGenerator(4375612914105986895L);
-		// show that not equal null
+		// show that not equal to null
 		for (int i = 0; i < 30; i++) {
 			PersonRange personRange = getRandomPersonRange(randomGenerator.nextLong());
 			assertFalse(personRange.equals(null));
 		}
 
-		// show that not equal another type
+		// show that not equal to another type
 		for (int i = 0; i < 30; i++) {
 			PersonRange personRange = getRandomPersonRange(randomGenerator.nextLong());
 			assertFalse(personRange.equals(new Object()));
@@ -137,35 +137,25 @@ public void testEquals() {
 			assertTrue(personRange.equals(personRange));
 		}
 
-		// symmetric, transitive
+		// symmetric, transitive, consistent
 		for (int i = 0; i < 30; i++) {
 			long seed = randomGenerator.nextLong();
 			PersonRange personRange = getRandomPersonRange(seed);
 			PersonRange duplicatePersonRange = getRandomPersonRange(seed);
-			assertTrue(personRange.equals(duplicatePersonRange));
-			assertTrue(duplicatePersonRange.equals(personRange));
-		}
-
-		// consistent
-		for (int i = 0; i < 30; i++) {
-			long seed = randomGenerator.nextLong();
-			PersonRange personRange = getRandomPersonRange(seed);
-			PersonRange duplicatePersonRange = getRandomPersonRange(seed);
-			for (int j = 0; j < 30; j++) {
-				assertTrue(personRange.equals(duplicatePersonRange));
-				assertTrue(personRange.equals(duplicatePersonRange));
-				assertTrue(personRange.equals(duplicatePersonRange));
+			assertFalse(personRange == duplicatePersonRange);
+			for (int j = 0; j < 10; j++) {		
 				assertTrue(personRange.equals(duplicatePersonRange));
+				assertTrue(duplicatePersonRange.equals(personRange));
 			}
 		}
 
-		// different inputs yield unequal plugin datas
+		// different inputs yield unequal person ranges
 		Set<PersonRange> set = new LinkedHashSet<>();
 		for (int i = 0; i < 100; i++) {
 			PersonRange personRange = getRandomPersonRange(randomGenerator.nextLong());
 			set.add(personRange);
 		}
-		assertEquals(100,set.size());
+		assertEquals(100, set.size());
 	}
 
 	@Test