Skip to content

Commit e5790ac

Browse files
zepfredzepfred
committed
chore: address comments
1 parent 7b60327 commit e5790ac

File tree

4 files changed

+53
-57
lines changed

4 files changed

+53
-57
lines changed

‎core/src/main/java/ai/timefold/solver/core/impl/domain/valuerange/ValueRangeCache.java‎

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,19 @@ public final class ValueRangeCache<Value_>
2626

2727
private final List<Value_> valuesWithFastRandomAccess;
2828
private final Set<Value_> valuesWithFastLookup;
29-
private final CacheType cacheType;
29+
private final boolean trustedValues;
3030

31-
private ValueRangeCache(int size, Set<Value_> emptyCacheSet, CacheType cacheType) {
31+
private ValueRangeCache(int size, Set<Value_> emptyCacheSet, boolean trustedValues) {
3232
this.valuesWithFastRandomAccess = new ArrayList<>(size);
3333
this.valuesWithFastLookup = emptyCacheSet;
34-
this.cacheType = cacheType;
34+
this.trustedValues = trustedValues;
3535
}
3636

37-
private ValueRangeCache(Collection<Value_> collection, Set<Value_> emptyCacheSet, CacheType cacheType) {
37+
private ValueRangeCache(Collection<Value_> collection, Set<Value_> emptyCacheSet, boolean trustedValues) {
3838
this.valuesWithFastRandomAccess = new ArrayList<>(collection);
3939
this.valuesWithFastLookup = emptyCacheSet;
4040
this.valuesWithFastLookup.addAll(valuesWithFastRandomAccess);
41-
this.cacheType = cacheType;
41+
this.trustedValues = trustedValues;
4242
}
4343

4444
public void add(@Nullable Value_ value) {
@@ -83,10 +83,11 @@ public Iterator<Value_> iterator(Random workingRandom) {
8383
*/
8484
public ValueRangeCache<Value_> sort(ValueRangeSorter<Value_> sorter) {
8585
var valuesWithFastRandomAccessSorted = sorter.sort(valuesWithFastRandomAccess);
86-
return switch (cacheType) {
87-
case USER_VALUES -> Builder.FOR_USER_VALUES.buildCache(valuesWithFastRandomAccessSorted);
88-
case TRUSTED_VALUES -> Builder.FOR_TRUSTED_VALUES.buildCache(valuesWithFastRandomAccessSorted);
89-
};
86+
if (trustedValues) {
87+
return Builder.FOR_TRUSTED_VALUES.buildCache(valuesWithFastRandomAccessSorted);
88+
} else {
89+
return Builder.FOR_USER_VALUES.buildCache(valuesWithFastRandomAccessSorted);
90+
}
9091
}
9192

9293
public enum Builder {
@@ -97,13 +98,12 @@ public enum Builder {
9798
FOR_USER_VALUES {
9899
@Override
99100
public <Value_> ValueRangeCache<Value_> buildCache(int size) {
100-
return new ValueRangeCache<>(size, CollectionUtils.newIdentityHashSet(size), CacheType.USER_VALUES);
101+
return new ValueRangeCache<>(size, CollectionUtils.newIdentityHashSet(size), false);
101102
}
102103

103104
@Override
104105
public <Value_> ValueRangeCache<Value_> buildCache(Collection<Value_> collection) {
105-
return new ValueRangeCache<>(collection, CollectionUtils.newIdentityHashSet(collection.size()),
106-
CacheType.USER_VALUES);
106+
return new ValueRangeCache<>(collection, CollectionUtils.newIdentityHashSet(collection.size()), false);
107107
}
108108

109109
},
@@ -118,13 +118,12 @@ public <Value_> ValueRangeCache<Value_> buildCache(Collection<Value_> collection
118118
FOR_TRUSTED_VALUES {
119119
@Override
120120
public <Value_> ValueRangeCache<Value_> buildCache(int size) {
121-
return new ValueRangeCache<>(size, CollectionUtils.newHashSet(size), CacheType.TRUSTED_VALUES);
121+
return new ValueRangeCache<>(size, CollectionUtils.newHashSet(size), true);
122122
}
123123

124124
@Override
125125
public <Value_> ValueRangeCache<Value_> buildCache(Collection<Value_> collection) {
126-
return new ValueRangeCache<>(collection, CollectionUtils.newHashSet(collection.size()),
127-
CacheType.TRUSTED_VALUES);
126+
return new ValueRangeCache<>(collection, CollectionUtils.newHashSet(collection.size()), true);
128127
}
129128

130129
};
@@ -135,9 +134,4 @@ public <Value_> ValueRangeCache<Value_> buildCache(Collection<Value_> collection
135134

136135
}
137136

138-
private enum CacheType {
139-
USER_VALUES,
140-
TRUSTED_VALUES
141-
}
142-
143137
}

‎core/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/value/FromEntityPropertyValueSelector.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,9 @@ public Iterator<Object> endingIterator(Object entity) {
123123
public boolean equals(Object o) {
124124
if (!(o instanceof FromEntityPropertyValueSelector<?> that))
125125
return false;
126-
return randomSelection == that.randomSelection && Objects.equals(valueRangeDescriptor, that.valueRangeDescriptor)
127-
&& Objects.equals(selectionSorter, that.selectionSorter);
126+
return Objects.equals(valueRangeDescriptor, that.valueRangeDescriptor)
127+
&& Objects.equals(selectionSorter, that.selectionSorter)
128+
&& randomSelection == that.randomSelection;
128129
}
129130

130131
@Override

‎core/src/main/java/ai/timefold/solver/core/impl/heuristic/selector/value/ValueSelectorFactory.java‎

Lines changed: 36 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package ai.timefold.solver.core.impl.heuristic.selector.value;
22

3+
import static ai.timefold.solver.core.config.heuristic.selector.common.SelectionOrder.SORTED;
4+
35
import java.util.ArrayList;
46
import java.util.Comparator;
57
import java.util.List;
@@ -273,37 +275,38 @@ private static Class<? extends ComparatorFactory> determineComparatorFactoryClas
273275

274276
private SelectionSorter<Solution_, Object> determineSorter(GenuineVariableDescriptor<Solution_> variableDescriptor,
275277
SelectionOrder resolvedSelectionOrder, ClassInstanceCache instanceCache) {
276-
SelectionSorter<Solution_, Object> sorter = null;
277-
if (resolvedSelectionOrder == ai.timefold.solver.core.config.heuristic.selector.common.SelectionOrder.SORTED) {
278-
var sorterManner = config.getSorterManner();
279-
var comparatorClass = determineComparatorClass(config);
280-
var comparatorFactoryClass = determineComparatorFactoryClass(config);
281-
if (sorterManner != null) {
282-
if (!ValueSelectorConfig.hasSorter(sorterManner, variableDescriptor)) {
283-
return null;
284-
}
285-
sorter = ValueSelectorConfig.determineSorter(sorterManner, variableDescriptor);
286-
} else if (comparatorClass != null) {
287-
Comparator<Object> sorterComparator =
288-
instanceCache.newInstance(config, determineComparatorPropertyName(config), comparatorClass);
289-
sorter = new ComparatorSelectionSorter<>(sorterComparator,
290-
SelectionSorterOrder.resolve(config.getSorterOrder()));
291-
} else if (comparatorFactoryClass != null) {
292-
var comparatorFactory = instanceCache.newInstance(config, determineComparatorFactoryPropertyName(config),
293-
comparatorFactoryClass);
294-
sorter = new ComparatorFactorySelectionSorter<>(comparatorFactory,
295-
SelectionSorterOrder.resolve(config.getSorterOrder()));
296-
} else if (config.getSorterClass() != null) {
297-
sorter = instanceCache.newInstance(config, "sorterClass", config.getSorterClass());
298-
} else {
299-
throw new IllegalArgumentException("""
300-
The valueSelectorConfig (%s) with resolvedSelectionOrder (%s) needs \
301-
a sorterManner (%s) or a %s (%s) or a %s (%s) \
302-
or a sorterClass (%s)."""
303-
.formatted(config, resolvedSelectionOrder, sorterManner, determineComparatorPropertyName(config),
304-
comparatorClass, determineComparatorFactoryPropertyName(config), comparatorFactoryClass,
305-
config.getSorterClass()));
278+
if (resolvedSelectionOrder != SORTED) {
279+
return null;
280+
}
281+
SelectionSorter<Solution_, Object> sorter;
282+
var sorterManner = config.getSorterManner();
283+
var comparatorClass = determineComparatorClass(config);
284+
var comparatorFactoryClass = determineComparatorFactoryClass(config);
285+
if (sorterManner != null) {
286+
if (!ValueSelectorConfig.hasSorter(sorterManner, variableDescriptor)) {
287+
return null;
306288
}
289+
sorter = ValueSelectorConfig.determineSorter(sorterManner, variableDescriptor);
290+
} else if (comparatorClass != null) {
291+
Comparator<Object> sorterComparator =
292+
instanceCache.newInstance(config, determineComparatorPropertyName(config), comparatorClass);
293+
sorter = new ComparatorSelectionSorter<>(sorterComparator,
294+
SelectionSorterOrder.resolve(config.getSorterOrder()));
295+
} else if (comparatorFactoryClass != null) {
296+
var comparatorFactory = instanceCache.newInstance(config, determineComparatorFactoryPropertyName(config),
297+
comparatorFactoryClass);
298+
sorter = new ComparatorFactorySelectionSorter<>(comparatorFactory,
299+
SelectionSorterOrder.resolve(config.getSorterOrder()));
300+
} else if (config.getSorterClass() != null) {
301+
sorter = instanceCache.newInstance(config, "sorterClass", config.getSorterClass());
302+
} else {
303+
throw new IllegalArgumentException("""
304+
The valueSelectorConfig (%s) with resolvedSelectionOrder (%s) needs \
305+
a sorterManner (%s) or a %s (%s) or a %s (%s) \
306+
or a sorterClass (%s)."""
307+
.formatted(config, resolvedSelectionOrder, sorterManner, determineComparatorPropertyName(config),
308+
comparatorClass, determineComparatorFactoryPropertyName(config), comparatorFactoryClass,
309+
config.getSorterClass()));
307310
}
308311
return sorter;
309312
}
@@ -372,14 +375,14 @@ protected void validateSorting(SelectionOrder resolvedSelectionOrder) {
372375
var sorterOrder = config.getSorterOrder();
373376
var sorterClass = config.getSorterClass();
374377
if ((sorterManner != null || comparatorClass != null || comparatorFactoryClass != null
375-
|| sorterOrder != null || sorterClass != null) && resolvedSelectionOrder != SelectionOrder.SORTED) {
378+
|| sorterOrder != null || sorterClass != null) && resolvedSelectionOrder != SORTED) {
376379
throw new IllegalArgumentException("""
377380
The valueSelectorConfig (%s) with sorterManner (%s) \
378381
and %s (%s) and %s (%s) and sorterOrder (%s) and sorterClass (%s) \
379382
has a resolvedSelectionOrder (%s) that is not %s."""
380383
.formatted(config, sorterManner, comparatorPropertyName, comparatorClass, comparatorFactoryPropertyName,
381384
comparatorFactoryClass, sorterOrder, sorterClass, resolvedSelectionOrder,
382-
SelectionOrder.SORTED));
385+
SORTED));
383386
}
384387
assertNotSorterMannerAnd(config, comparatorPropertyName, ValueSelectorFactory::determineComparatorClass);
385388
assertNotSorterMannerAnd(config, comparatorFactoryPropertyName,
@@ -421,7 +424,7 @@ private static void assertNotSorterClassAnd(ValueSelectorConfig config, String p
421424

422425
protected ValueSelector<Solution_> applySorting(SelectionCacheType resolvedCacheType, SelectionOrder resolvedSelectionOrder,
423426
ValueSelector<Solution_> valueSelector, ClassInstanceCache instanceCache) {
424-
if (resolvedSelectionOrder == SelectionOrder.SORTED) {
427+
if (resolvedSelectionOrder == SORTED) {
425428
var sorter = determineSorter(valueSelector.getVariableDescriptor(), resolvedSelectionOrder, instanceCache);
426429
if (!valueSelector.getVariableDescriptor().canExtractValueRangeFromSolution()
427430
&& resolvedCacheType == SelectionCacheType.STEP) {

‎core/src/main/java/ai/timefold/solver/core/impl/score/director/ValueRangeManager.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,6 @@ public <T> CountableValueRange<T> getFromSolution(ValueRangeDescriptor<Solution_
381381
var valueRange =
382382
item != null ? (CountableValueRange<T>) item.countableValueRange() : null;
383383
var valueRangeSorter = item != null ? item.sorter() : null;
384-
// Avoid computeIfAbsent on the hot path; creates capturing lambda instances.
385384
// We read and sort the data again if needed
386385
if (valueRange == null || (sorter != null && !Objects.equals(valueRangeSorter, sorter))) {
387386
var extractedValueRange = valueRangeDescriptor.<T> extractAllValues(Objects.requireNonNull(solution));
@@ -435,7 +434,6 @@ public <T> CountableValueRange<T> getFromEntity(ValueRangeDescriptor<Solution_>
435434
var item = valueRangeList[valueRangeDescriptor.getOrdinal()];
436435
var valueRange = item != null ? (CountableValueRange<T>) item.countableValueRange() : null;
437436
var valueRangeSorter = item != null ? item.sorter() : null;
438-
// Avoid computeIfAbsent on the hot path; creates capturing lambda instances.
439437
// We read and sort the data again if needed
440438
if (valueRange == null || (sorter != null && !Objects.equals(valueRangeSorter, sorter))) {
441439
var extractedValueRange =

0 commit comments

Comments
 (0)