WIP

Author: triceo

core/src/main/java/ai/timefold/solver/core/api/domain/valuerange/ValueRangeProvider.java

@@ -23,6 +23,12 @@
  * A {@link Collection} is implicitly converted to a {@link ValueRange}.
  *
  * <p>
+ * If two values in a value range are equal according to {@link Object#equals(Object)},
+ * then they are considered the same value
+ * and will only be present once in the value range,
+ * regardless of how many times they are present originally.
+ *
+ * <p>
  * Value ranges are not allowed to change during solving.
  * This is especially important for value ranges defined on {@link PlanningEntity}-annotated classes;
  * these must never depend on any of that entity's variables, or on any other entity's variables.

core/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/descriptor/AbstractFromPropertyValueRangeDescriptor.java

@@ -1,9 +1,9 @@
 package ai.timefold.solver.core.impl.domain.valuerange.descriptor;
 
 import java.lang.reflect.Array;
-import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
-import java.util.LinkedList;
+import java.util.Collections;
 import java.util.List;
 
 import ai.timefold.solver.core.api.domain.solution.PlanningSolution;
@@ -12,7 +12,6 @@
 import ai.timefold.solver.core.api.domain.valuerange.ValueRangeProvider;
 import ai.timefold.solver.core.api.domain.variable.PlanningVariable;
 import ai.timefold.solver.core.config.util.ConfigUtils;
-import ai.timefold.solver.core.impl.domain.common.ReflectionHelper;
 import ai.timefold.solver.core.impl.domain.common.accessor.MemberAccessor;
 import ai.timefold.solver.core.impl.domain.entity.descriptor.EntityDescriptor;
 import ai.timefold.solver.core.impl.domain.valuerange.buildin.collection.ListValueRange;
@@ -105,8 +104,8 @@ protected <Value_> ValueRange<Value_> readValueRange(Object bean) {
         }
         ValueRange<Value_> valueRange;
         if (collectionWrapping || arrayWrapping) {
-            List<Value_> list = collectionWrapping ? transformCollectionToList((Collection<Value_>) valueRangeObject)
-                    : ReflectionHelper.transformArrayToList(valueRangeObject);
+            List<Value_> list = collectionWrapping ? transformCollectionToUniqueList((Collection<Value_>) valueRangeObject)
+                    : transformArrayToUniqueList(valueRangeObject);
             // Don't check the entire list for performance reasons, but do check common pitfalls
             if (!list.isEmpty() && (list.get(0) == null || list.get(list.size() - 1) == null)) {
                 throw new IllegalStateException(
@@ -163,18 +162,27 @@ protected long readValueRangeSize(Object bean) {
         }
     }
 
-    private <T> List<T> transformCollectionToList(Collection<T> collection) {
-        if (collection instanceof List<T> list) {
-            if (collection instanceof LinkedList<T> linkedList) {
-                // ValueRange.createRandomIterator(Random) and ValueRange.get(int) wouldn't be efficient.
-                return new ArrayList<>(linkedList);
-            } else {
-                return list;
-            }
-        } else {
-            // TODO If only ValueRange.createOriginalIterator() is used, cloning a Set to a List is a waste of time.
-            return new ArrayList<>(collection);
+    private <T> List<T> transformCollectionToUniqueList(Collection<T> collection) {
+        if (collection.isEmpty()) {
+            return Collections.emptyList();
+        }
+        return collection.stream()
+                .distinct()
+                .toList();
+    }
+
+    @SuppressWarnings("unchecked")
+    public static <Value_> List<Value_> transformArrayToUniqueList(Object arrayObject) {
+        if (arrayObject == null) {
+            return Collections.emptyList();
+        }
+        var array = (Value_[]) arrayObject;
+        if (array.length == 0) {
+            return Collections.emptyList();
         }
+        return Arrays.stream(array)
+                .distinct()
+                .toList();
     }
 
 }

core/src/test/java/ai/timefold/solver/core/impl/domain/solution/cloner/AbstractSolutionClonerTest.java

@@ -55,9 +55,9 @@
 import ai.timefold.solver.core.testdomain.inheritance.solution.baseannotated.thirdparty.TestdataExtendedThirdPartyEntity;
 import ai.timefold.solver.core.testdomain.inheritance.solution.baseannotated.thirdparty.TestdataExtendedThirdPartySolution;
 import ai.timefold.solver.core.testdomain.inheritance.solution.baseannotated.thirdparty.TestdataThirdPartyEntityPojo;
-import ai.timefold.solver.core.testdomain.list.externalized.TestdataListEntityExternalized;
-import ai.timefold.solver.core.testdomain.list.externalized.TestdataListSolutionExternalized;
-import ai.timefold.solver.core.testdomain.list.externalized.TestdataListValueExternalized;
+import ai.timefold.solver.core.testdomain.list.TestdataListEntity;
+import ai.timefold.solver.core.testdomain.list.TestdataListSolution;
+import ai.timefold.solver.core.testdomain.list.TestdataListValue;
 import ai.timefold.solver.core.testdomain.reflect.accessmodifier.TestdataAccessModifierSolution;
 import ai.timefold.solver.core.testdomain.reflect.field.TestdataFieldAnnotatedEntity;
 import ai.timefold.solver.core.testdomain.reflect.field.TestdataFieldAnnotatedSolution;
@@ -118,19 +118,18 @@ void cloneSolution() {
 
     @Test
     void cloneListVariableSolution() {
-        var solutionDescriptor = SolutionDescriptor.buildSolutionDescriptor(
-                TestdataListSolutionExternalized.class,
-                TestdataListEntityExternalized.class);
+        var solutionDescriptor =
+                SolutionDescriptor.buildSolutionDescriptor(TestdataListSolution.class, TestdataListEntity.class);
 
         var cloner = createSolutionCloner(solutionDescriptor);
 
-        var val1 = new TestdataListValueExternalized("1");
-        var val2 = new TestdataListValueExternalized("2");
-        var val3 = new TestdataListValueExternalized("3");
-        var a = new TestdataListEntityExternalized("a", new ArrayList<>(List.of(val1, val3)));
-        var b = new TestdataListEntityExternalized("b", new ArrayList<>(List.of(val2)));
+        var val1 = new TestdataListValue("1");
+        var val2 = new TestdataListValue("2");
+        var val3 = new TestdataListValue("3");
+        var a = new TestdataListEntity("a", new ArrayList<>(List.of(val1, val3)));
+        var b = new TestdataListEntity("b", new ArrayList<>(List.of(val2)));
 
-        var original = new TestdataListSolutionExternalized();
+        var original = new TestdataListSolution();
         var valueList = Arrays.asList(val1, val2, val3);
         original.setValueList(valueList);
         var originalEntityList = List.of(a, b);
@@ -140,7 +139,7 @@ void cloneListVariableSolution() {
         var clone = cloner.cloneSolution(original);
 
         assertThat(clone).isNotSameAs(original);
-        assertThat(clone.getValueList()).isSameAs(valueList);
+        assertThat(clone.getValueList()).isEqualTo(valueList);
         assertThat(clone.getScore()).isEqualTo(original.getScore());
 
         var cloneEntityList = clone.getEntityList();
@@ -410,8 +409,8 @@ private void assertEntityClone(TestdataThirdPartyEntityPojo originalEntity,
         assertCode(valueCode, cloneEntity.getValue());
     }
 
-    private void assertEntityListClone(TestdataListEntityExternalized originalEntity,
-            TestdataListEntityExternalized cloneEntity, String entityCode, List<String> valueCodeList) {
+    private void assertEntityListClone(TestdataListEntity originalEntity, TestdataListEntity cloneEntity, String entityCode,
+            List<String> valueCodeList) {
         assertThat(cloneEntity).isNotSameAs(originalEntity);
         assertThat(cloneEntity.getValueList()).isNotSameAs(originalEntity.getValueList());
         assertCode(entityCode, cloneEntity);

core/src/test/java/ai/timefold/solver/core/impl/localsearch/DefaultLocalSearchPhaseTest.java

@@ -28,8 +28,6 @@
 import ai.timefold.solver.core.testdomain.list.TestdataListEntity;
 import ai.timefold.solver.core.testdomain.list.TestdataListSolution;
 import ai.timefold.solver.core.testdomain.list.TestdataListValue;
-import ai.timefold.solver.core.testdomain.list.externalized.TestdataListEntityExternalized;
-import ai.timefold.solver.core.testdomain.list.externalized.TestdataListSolutionExternalized;
 import ai.timefold.solver.core.testdomain.pinned.TestdataPinnedEntity;
 import ai.timefold.solver.core.testdomain.pinned.TestdataPinnedSolution;
 import ai.timefold.solver.core.testdomain.pinned.unassignedvar.TestdataPinnedAllowsUnassignedEntity;
@@ -370,17 +368,6 @@ void solveMultiVarChainedVariable() {
         assertThat(solution).isNotNull();
     }
 
-    @Test
-    void solveListVariableWithExternalizedInverseAndIndexSupplies() {
-        var solverConfig = PlannerTestUtils.buildSolverConfig(
-                TestdataListSolutionExternalized.class, TestdataListEntityExternalized.class);
-
-        var solution = TestdataListSolutionExternalized.generateUninitializedSolution(6, 2);
-
-        solution = PlannerTestUtils.solve(solverConfig, solution);
-        assertThat(solution).isNotNull();
-    }
-
     @Test
     void failsFastWithUninitializedSolutionBasicVariable() {
         var solverConfig = PlannerTestUtils.buildSolverConfig(TestdataSolution.class, TestdataEntity.class);
@@ -392,17 +379,4 @@ void failsFastWithUninitializedSolutionBasicVariable() {
                 .hasMessageContaining("uninitialized entities");
     }
 
-    @Test
-    void failsFastWithUninitializedSolutionListVariable() {
-        var solverConfig = PlannerTestUtils.buildSolverConfig(
-                TestdataListSolutionExternalized.class, TestdataListEntityExternalized.class);
-        solverConfig.withPhases(solverConfig.getPhaseConfigList().get(1)); // Remove construction heuristic.
-
-        var solution = TestdataListSolutionExternalized.generateUninitializedSolution(6, 2);
-
-        assertThatThrownBy(() -> PlannerTestUtils.solve(solverConfig, solution))
-                .hasMessageContaining("planning list variable")
-                .hasMessageContaining("unexpected unassigned values");
-    }
-
 }

core/src/test/java/ai/timefold/solver/core/testdomain/list/TestdataListEntity.java

@@ -54,6 +54,10 @@ public List<TestdataListValue> getValueList() {
         return valueList;
     }
 
+    public void setValueList(List<TestdataListValue> valueList) {
+        this.valueList = valueList;
+    }
+
     public void addValue(TestdataListValue value) {
         addValueAt(valueList.size(), value);
     }