Skip to content

Commit

Permalink
Sonar
Browse files Browse the repository at this point in the history
  • Loading branch information
triceo committed Nov 7, 2024
1 parent 32d9ff8 commit 21055e3
Show file tree
Hide file tree
Showing 11 changed files with 111 additions and 145 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ public abstract class AbstractScoreDirector<Solution_, Score_ extends Score<Scor

private final boolean lookUpEnabled;
private final LookUpManager lookUpManager;
protected final ConstraintMatchPolicy constraintMatchPolicy;
private final boolean expectShadowVariablesInCorrectState;
protected final Factory_ scoreDirectorFactory;
private final VariableDescriptorCache<Solution_> variableDescriptorCache;
protected final VariableListenerSupport<Solution_> variableListenerSupport;
protected final ConstraintMatchPolicy constraintMatchPolicy;

private long workingEntityListRevision = 0L;
private int workingGenuineEntityCount = 0;
Expand All @@ -93,12 +93,12 @@ protected AbstractScoreDirector(Factory_ scoreDirectorFactory, boolean lookUpEna
this.lookUpManager = lookUpEnabled
? new LookUpManager(solutionDescriptor.getLookUpStrategyResolver())
: null;
this.constraintMatchPolicy = constraintMatchPolicy;
this.expectShadowVariablesInCorrectState = expectShadowVariablesInCorrectState;
this.scoreDirectorFactory = scoreDirectorFactory;
this.variableDescriptorCache = new VariableDescriptorCache<>(solutionDescriptor);
this.variableListenerSupport = VariableListenerSupport.create(this);
this.variableListenerSupport.linkVariableListeners();
this.constraintMatchPolicy = constraintMatchPolicy;
if (scoreDirectorFactory.isTrackingWorkingSolution()) {
this.solutionTracker = new SolutionTracker<>(getSolutionDescriptor(),
getSupplyManager());
Expand All @@ -115,6 +115,11 @@ protected AbstractScoreDirector(Factory_ scoreDirectorFactory, boolean lookUpEna
}
}

@Override
public final ConstraintMatchPolicy getConstraintMatchPolicy() {
return constraintMatchPolicy;
}

@Override
public Factory_ getScoreDirectorFactory() {
return scoreDirectorFactory;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public void setTrackingWorkingSolution(boolean trackingWorkingSolution) {

@Override
public InnerScoreDirector<Solution_, Score_> buildScoreDirector() {
return buildScoreDirector(true, ConstraintMatchPolicy.ENABLED);
return buildScoreDirector(false, ConstraintMatchPolicy.DISABLED);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ public interface InnerScoreDirectorFactory<Solution_, Score_ extends Score<Score
InnerScoreDirector<Solution_, Score_> buildScoreDirector();

/**
* Like {@link #buildScoreDirector()}, but optionally disables {@link ConstraintMatch} tracking and look up
* for more performance (presuming the {@link ScoreDirector} implementation actually supports it to begin with).
* Like {@link #buildScoreDirector()}, but optionally enables {@link ConstraintMatch} tracking and look up
* where possible and necessary.
*
* @param lookUpEnabled true if a {@link ScoreDirector} implementation should track all working objects
* for {@link ScoreDirector#lookUpWorkingObject(Object)}
Expand All @@ -46,8 +46,8 @@ default InnerScoreDirector<Solution_, Score_> buildScoreDirector(boolean lookUpE
}

/**
* Like {@link #buildScoreDirector()}, but optionally disables {@link ConstraintMatch} tracking and look up
* for more performance (presuming the {@link ScoreDirector} implementation actually supports it to begin with).
* Like {@link #buildScoreDirector()}, but optionally enables {@link ConstraintMatch} tracking and look up
* where possible and necessary.
*
* @param lookUpEnabled true if a {@link ScoreDirector} implementation should track all working objects
* for {@link ScoreDirector#lookUpWorkingObject(Object)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ public final class EasyScoreDirector<Solution_, Score_ extends Score<Score_>>
private final EasyScoreCalculator<Solution_, Score_> easyScoreCalculator;

public EasyScoreDirector(EasyScoreDirectorFactory<Solution_, Score_> scoreDirectorFactory,
boolean lookUpEnabled, ConstraintMatchPolicy constraintMatchPolicy, boolean expectShadowVariablesInCorrectState,
boolean lookUpEnabled, boolean expectShadowVariablesInCorrectState,
EasyScoreCalculator<Solution_, Score_> easyScoreCalculator) {
super(scoreDirectorFactory, lookUpEnabled, constraintMatchPolicy, expectShadowVariablesInCorrectState);
super(scoreDirectorFactory, lookUpEnabled, ConstraintMatchPolicy.DISABLED, expectShadowVariablesInCorrectState);
this.easyScoreCalculator = easyScoreCalculator;
}

Expand Down Expand Up @@ -64,16 +64,6 @@ public Score_ calculateScore() {
return score;
}

/**
* Always false, {@link ConstraintMatchTotal}s are not supported by this {@link ScoreDirector} implementation.
*
* @return false
*/
@Override
public ConstraintMatchPolicy getConstraintMatchPolicy() {
return ConstraintMatchPolicy.DISABLED;
}

/**
* {@link ConstraintMatch}s are not supported by this {@link ScoreDirector} implementation.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,9 @@ public EasyScoreDirectorFactory(SolutionDescriptor<Solution_> solutionDescriptor
// ************************************************************************

@Override
public EasyScoreDirector<Solution_, Score_> buildScoreDirector(
boolean lookUpEnabled, ConstraintMatchPolicy constraintMatchPolicy, boolean expectShadowVariablesInCorrectState) {
return new EasyScoreDirector<>(this, lookUpEnabled, constraintMatchPolicy, expectShadowVariablesInCorrectState,
easyScoreCalculator);
public EasyScoreDirector<Solution_, Score_> buildScoreDirector(boolean lookUpEnabled,
ConstraintMatchPolicy constraintMatchPolicy, boolean expectShadowVariablesInCorrectState) {
return new EasyScoreDirector<>(this, lookUpEnabled, expectShadowVariablesInCorrectState, easyScoreCalculator);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,21 @@ public final class IncrementalScoreDirector<Solution_, Score_ extends Score<Scor
public IncrementalScoreDirector(IncrementalScoreDirectorFactory<Solution_, Score_> scoreDirectorFactory,
boolean lookUpEnabled, ConstraintMatchPolicy constraintMatchPolicy, boolean expectShadowVariablesInCorrectState,
IncrementalScoreCalculator<Solution_, Score_> incrementalScoreCalculator) {
super(scoreDirectorFactory, lookUpEnabled, constraintMatchPolicy, expectShadowVariablesInCorrectState);
super(scoreDirectorFactory, lookUpEnabled,
determineCorrectPolicy(constraintMatchPolicy, incrementalScoreCalculator),
expectShadowVariablesInCorrectState);
this.incrementalScoreCalculator = incrementalScoreCalculator;
}

private static ConstraintMatchPolicy determineCorrectPolicy(ConstraintMatchPolicy constraintMatchPolicy,
IncrementalScoreCalculator<?, ?> incrementalScoreCalculator) {
if (incrementalScoreCalculator instanceof ConstraintMatchAwareIncrementalScoreCalculator<?, ?>) {
return constraintMatchPolicy;
} else {
return ConstraintMatchPolicy.DISABLED;
}
}

public IncrementalScoreCalculator<Solution_, Score_> getIncrementalScoreCalculator() {
return incrementalScoreCalculator;
}
Expand All @@ -65,11 +76,10 @@ public void setWorkingSolution(Solution_ workingSolution) {
@Override
public Score_ calculateScore() {
variableListenerSupport.assertNotificationQueuesAreEmpty();
Score_ score = incrementalScoreCalculator.calculateScore();
if (score == null) {
throw new IllegalStateException("The incrementalScoreCalculator (" + incrementalScoreCalculator.getClass()
+ ") must return a non-null score (" + score + ") in the method calculateScore().");
} else if (!score.isSolutionInitialized()) {
Score_ score = Objects.requireNonNull(incrementalScoreCalculator.calculateScore(),
() -> "The incrementalScoreCalculator (%s) must return a non-null score in the method calculateScore()."
.formatted(incrementalScoreCalculator));
if (!score.isSolutionInitialized()) {
throw new IllegalStateException("The score (" + this + ")'s initScore (" + score.initScore()
+ ") should be 0.\n"
+ "Maybe the score calculator (" + incrementalScoreCalculator.getClass() + ") is calculating "
Expand All @@ -83,19 +93,9 @@ public Score_ calculateScore() {
return score;
}

@Override
public ConstraintMatchPolicy getConstraintMatchPolicy() {
if (!constraintMatchPolicy.isEnabled()) {
return ConstraintMatchPolicy.DISABLED;
}
return incrementalScoreCalculator instanceof ConstraintMatchAwareIncrementalScoreCalculator
? constraintMatchPolicy
: ConstraintMatchPolicy.DISABLED;
}

@Override
public Map<String, ConstraintMatchTotal<Score_>> getConstraintMatchTotalMap() {
if (!getConstraintMatchPolicy().isEnabled()) {
if (!constraintMatchPolicy.isEnabled()) {
throw new IllegalStateException("When constraint matching (" + constraintMatchPolicy
+ ") is disabled in the constructor, this method should not be called.");
}
Expand All @@ -108,7 +108,7 @@ public Map<String, ConstraintMatchTotal<Score_>> getConstraintMatchTotalMap() {

@Override
public Map<Object, Indictment<Score_>> getIndictmentMap() {
if (!getConstraintMatchPolicy().isJustificationEnabled()) {
if (!constraintMatchPolicy.isJustificationEnabled()) {
throw new IllegalStateException("When constraint matching with justifications (" + constraintMatchPolicy
+ ") is disabled in the constructor, this method should not be called.");
}
Expand All @@ -118,7 +118,7 @@ public Map<Object, Indictment<Score_>> getIndictmentMap() {
if (incrementalIndictmentMap != null) {
return incrementalIndictmentMap;
}
Map<Object, Indictment<Score_>> indictmentMap = new LinkedHashMap<>(); // TODO use entitySize
Map<Object, Indictment<Score_>> indictmentMap = new LinkedHashMap<>();
Score_ zeroScore = getScoreDefinition().getZeroScore();
Map<String, ConstraintMatchTotal<Score_>> constraintMatchTotalMap = getConstraintMatchTotalMap();
for (ConstraintMatchTotal<Score_> constraintMatchTotal : constraintMatchTotalMap.values()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,6 @@ public Score_ calculateScore() {
return score;
}

@Override
public ConstraintMatchPolicy getConstraintMatchPolicy() {
return constraintMatchPolicy;
}

@Override
public Map<String, ConstraintMatchTotal<Score_>> getConstraintMatchTotalMap() {
if (workingSolution == null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ai.timefold.solver.core.impl.score.stream.common.inliner;

import java.lang.reflect.InvocationTargetException;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.TreeMap;
Expand Down Expand Up @@ -30,7 +31,6 @@
import ai.timefold.solver.core.impl.score.stream.common.AbstractConstraint;
import ai.timefold.solver.core.impl.util.CollectionUtils;
import ai.timefold.solver.core.impl.util.ElementAwareList;
import ai.timefold.solver.core.impl.util.ElementAwareListEntry;

/**
* Keeps track of the working score and constraint matches for a single constraint session.
Expand Down Expand Up @@ -76,7 +76,7 @@ public abstract class AbstractScoreInliner<Score_ extends Score<Score_>> {
return (ScoreInliner_) new BendableBigDecimalScoreInliner((Map) constraintWeightMap, constraintMatchPolicy,
bendableScoreDefinition.getHardLevelsSize(), bendableScoreDefinition.getSoftLevelsSize());
} else {
String customScoreInlinerClassName = System.getProperty(CUSTOM_SCORE_INLINER_CLASS_PROPERTY_NAME);
var customScoreInlinerClassName = System.getProperty(CUSTOM_SCORE_INLINER_CLASS_PROPERTY_NAME);
if (customScoreInlinerClassName == null) {
throw new UnsupportedOperationException("Unknown score definition class (" +
scoreDefinition.getClass().getCanonicalName() + ").\n" +
Expand All @@ -86,7 +86,7 @@ public abstract class AbstractScoreInliner<Score_ extends Score<Score_>> {
"Note: support for custom scores will be removed in Timefold 2.0.");
}
try {
Class<?> customScoreInlinerClass = Class.forName(customScoreInlinerClassName);
var customScoreInlinerClass = Class.forName(customScoreInlinerClassName);
if (!AbstractScoreInliner.class.isAssignableFrom(customScoreInlinerClass)) {
throw new IllegalStateException("Custom score inliner class (" + customScoreInlinerClassName +
") does not extend " + AbstractScoreInliner.class.getCanonicalName() + ".\n" +
Expand Down Expand Up @@ -114,13 +114,14 @@ protected AbstractScoreInliner(Map<Constraint, Score_> constraintWeightMap, Cons
this.constraintMatchPolicy = constraintMatchPolicy;
constraintWeightMap.forEach(this::validateConstraintWeight);
this.constraintWeightMap = constraintWeightMap;
this.constraintMatchMap =
constraintMatchPolicy.isEnabled() ? CollectionUtils.newIdentityHashMap(constraintWeightMap.size()) : null;
if (constraintMatchPolicy.isEnabled()) {
this.constraintMatchMap = CollectionUtils.newIdentityHashMap(constraintWeightMap.size());
for (var constraint : constraintWeightMap.keySet()) {
// Ensure that even constraints without matches have their entry.
constraintMatchMap.put(constraint, new ElementAwareList<>());
this.constraintMatchMap.put(constraint, new ElementAwareList<>());
}
} else {
this.constraintMatchMap = Collections.emptyMap();
}
}

Expand All @@ -144,12 +145,12 @@ private void validateConstraintWeight(Constraint constraint, Score_ constraintWe

protected final UndoScoreImpacter addConstraintMatch(Constraint constraint, Score_ score,
ConstraintMatchSupplier<Score_> constraintMatchSupplier, UndoScoreImpacter undoScoreImpact) {
ElementAwareList<ConstraintMatchCarrier<Score_>> constraintMatchList = getConstraintMatchList(constraint);
var constraintMatchList = getConstraintMatchList(constraint);
/*
* Creating a constraint match is a heavy operation which may yet be undone.
* Defer creation of the constraint match until a later point.
*/
ElementAwareListEntry<ConstraintMatchCarrier<Score_>> entry =
var entry =
constraintMatchList.add(new ConstraintMatchCarrier<>(constraintMatchSupplier, constraint, score));
clearMaps();
return () -> {
Expand All @@ -161,7 +162,7 @@ protected final UndoScoreImpacter addConstraintMatch(Constraint constraint, Scor

private ElementAwareList<ConstraintMatchCarrier<Score_>> getConstraintMatchList(Constraint constraint) {
// Optimization: computeIfAbsent() would have created a lambda on the hot path.
ElementAwareList<ConstraintMatchCarrier<Score_>> constraintMatchList = constraintMatchMap.get(constraint);
var constraintMatchList = constraintMatchMap.get(constraint);
if (constraintMatchList == null) {
throw new IllegalStateException(
"Impossible state: Unknown constraint (%s)."
Expand All @@ -180,7 +181,9 @@ public ConstraintMatchPolicy getConstraintMatchPolicy() {
}

public final Map<String, ConstraintMatchTotal<Score_>> getConstraintIdToConstraintMatchTotalMap() {
if (constraintIdToConstraintMatchTotalMap == null) {
if (!constraintMatchPolicy.isEnabled()) {
throw new IllegalStateException("Impossible state: Method called while constraint matching is disabled.");
} else if (constraintIdToConstraintMatchTotalMap == null) {
rebuildConstraintMatchTotals();
}
return constraintIdToConstraintMatchTotalMap;
Expand All @@ -203,7 +206,9 @@ private void rebuildConstraintMatchTotals() {
}

public final Map<Object, Indictment<Score_>> getIndictmentMap() {
if (indictmentMap == null) {
if (!constraintMatchPolicy.isJustificationEnabled()) {
throw new IllegalStateException("Impossible state: Method called while justifications are disabled.");
} else if (indictmentMap == null) {
rebuildIndictments();
}
return indictmentMap;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ public <Score_ extends Score<Score_>> InnerScoreDirectorFactory<Solution_, Score
metricsRequiringConstraintMatchSet);
}

var castScoreDirector = scoreDirectorFactory.buildScoreDirector(true, ConstraintMatchPolicy.ENABLED);
var castScoreDirector = scoreDirectorFactory.buildScoreDirector(true,
constraintMatchEnabled ? ConstraintMatchPolicy.ENABLED : ConstraintMatchPolicy.DISABLED);
solverScope.setScoreDirector(castScoreDirector);
solverScope.setProblemChangeDirector(new DefaultProblemChangeDirector<>(castScoreDirector));

Expand Down
Loading

0 comments on commit 21055e3

Please sign in to comment.