Skip to content

Commit 1a3fd22

Browse files
committed
fix: addressing PR comments
1 parent 74eee2b commit 1a3fd22

File tree

3 files changed

+45
-39
lines changed

3 files changed

+45
-39
lines changed

core/src/main/java/ai/timefold/solver/core/api/score/analysis/ConstraintAnalysis.java

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import java.util.List;
99
import java.util.Map;
1010
import java.util.Objects;
11-
import java.util.stream.Collectors;
1211
import java.util.stream.Stream;
1312

1413
import ai.timefold.solver.core.api.score.Score;
@@ -60,6 +59,11 @@ static <Score_ extends Score<Score_>> ConstraintAnalysis<Score_> of(ConstraintRe
6059
Objects.requireNonNull(score);
6160
}
6261

62+
/**
63+
* Return the match count of the constraint.
64+
*
65+
* @throws IllegalStateException if the {@link ConstraintAnalysis#matches()} is null
66+
*/
6367
public int matchCount() {
6468
if (matches == null) {
6569
throw new IllegalArgumentException("""
@@ -86,9 +90,9 @@ static <Score_ extends Score<Score_>> ConstraintAnalysis<Score_> diff(
8690
ConstraintAnalysis<Score_> otherConstraintAnalysis) {
8791
if (constraintAnalysis == null) {
8892
if (otherConstraintAnalysis == null) {
89-
throw new IllegalStateException("""
90-
Impossible state: none of the score explanations provided constraint matches for a constraint (%s).
91-
""".formatted(constraintRef));
93+
throw new IllegalStateException(
94+
"Impossible state: none of the score explanations provided constraint matches for a constraint (%s)."
95+
.formatted(constraintRef));
9296
}
9397
// No need to compute diff; this constraint is not present in this score explanation.
9498
return otherConstraintAnalysis.negate();
@@ -99,9 +103,9 @@ static <Score_ extends Score<Score_>> ConstraintAnalysis<Score_> diff(
99103
var matchAnalyses = constraintAnalysis.matches();
100104
var otherMatchAnalyses = otherConstraintAnalysis.matches();
101105
if ((matchAnalyses == null && otherMatchAnalyses != null) || (matchAnalyses != null && otherMatchAnalyses == null)) {
102-
throw new IllegalStateException("""
103-
Impossible state: Only one of the score analyses (%s, %s) provided match analyses for a constraint (%s)."""
104-
.formatted(constraintAnalysis, otherConstraintAnalysis, constraintRef));
106+
throw new IllegalStateException(
107+
"Impossible state: Only one of the score analyses (%s, %s) provided match analyses for a constraint (%s)."
108+
.formatted(constraintAnalysis, otherConstraintAnalysis, constraintRef));
105109
}
106110
// Compute the diff.
107111
var constraintWeightDifference = constraintAnalysis.weight().subtract(otherConstraintAnalysis.weight());
@@ -118,9 +122,9 @@ static <Score_ extends Score<Score_>> ConstraintAnalysis<Score_> diff(
118122
var otherMatchAnalysis = otherMatchAnalysisMap.get(justification);
119123
if (matchAnalysis == null) {
120124
if (otherMatchAnalysis == null) {
121-
throw new IllegalStateException("""
122-
Impossible state: none of the match analyses provided for a constraint (%s).
123-
""".formatted(constraintRef));
125+
throw new IllegalStateException(
126+
"Impossible state: none of the match analyses provided for a constraint (%s)."
127+
.formatted(constraintRef));
124128
}
125129
// No need to compute diff; this match is not present in this score explanation.
126130
return otherMatchAnalysis.negate();
@@ -132,7 +136,7 @@ static <Score_ extends Score<Score_>> ConstraintAnalysis<Score_> diff(
132136
justification);
133137
}
134138
})
135-
.collect(Collectors.toList());
139+
.toList();
136140
return new ConstraintAnalysis<>(constraintRef, constraintWeightDifference, scoreDifference, result);
137141
}
138142

@@ -172,11 +176,12 @@ public String constraintName() {
172176

173177
/**
174178
* Returns a diagnostic text that explains part of the score quality through the {@link ConstraintAnalysis} API.
179+
* The string is built fresh every time the method is called.
175180
*
176181
* @return never null
177182
*/
178183
public String summarize() {
179-
StringBuilder summary = new StringBuilder();
184+
var summary = new StringBuilder();
180185
summary.append("""
181186
Explanation of score (%s):
182187
Constraint matches:
@@ -191,25 +196,20 @@ Explanation of score (%s):
191196
""");
192197
}
193198
if (constraintMatches.isEmpty()) {
194-
summary.append("""
195-
%s: constraint (%s) has no matches.
196-
""".formatted(score().toShortString(), constraintRef().constraintName()));
199+
summary.append(
200+
"%8s%s: constraint (%s) has no matches.\n".formatted(" ", score().toShortString(),
201+
constraintRef().constraintName()));
197202
} else {
198-
summary.append("""
199-
%s: constraint (%s) has %s matches:
200-
""".formatted(score().toShortString(), constraintRef().constraintName(), constraintMatches.size()));
203+
summary.append("%8s%s: constraint (%s) has %s matches:\n".formatted(" ", score().toShortString(),
204+
constraintRef().constraintName(), constraintMatches.size()));
201205
}
202206
constraintMatches.stream()
203207
.sorted(matchScoreComparator)
204208
.limit(DEFAULT_SUMMARY_CONSTRAINT_MATCH_LIMIT)
205-
.forEach(match -> summary.append("""
206-
%s: justified with (%s)
207-
""".formatted(match.score().toShortString(),
209+
.forEach(match -> summary.append("%12S%s: justified with (%s)\n".formatted(" ", match.score().toShortString(),
208210
match.justification())));
209211
if (constraintMatches.size() > DEFAULT_SUMMARY_CONSTRAINT_MATCH_LIMIT) {
210-
summary.append("""
211-
...
212-
""");
212+
summary.append("%12s%s\n".formatted(" ", "..."));
213213
}
214214

215215
return summary.toString();

core/src/main/java/ai/timefold/solver/core/api/score/analysis/ScoreAnalysis.java

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ public Collection<ConstraintAnalysis<Score_>> constraintAnalyses() {
148148
/**
149149
* Returns a diagnostic text that explains the solution through the {@link ConstraintAnalysis} API to identify which
150150
* constraints cause that score quality.
151+
* The string is built fresh every time the method is called.
151152
* <p>
152153
* In case of an {@link Score#isFeasible() infeasible} solution, this can help diagnose the cause of that.
153154
*
@@ -179,29 +180,34 @@ Explanation of score (%s):
179180
""");
180181
}
181182
if (matches.isEmpty()) {
182-
summary.append("""
183-
%s: constraint (%s) has no matches.
184-
""".formatted(constraint.score().toShortString(), constraint.constraintRef().constraintName()));
183+
summary.append(
184+
"%8s%s: constraint (%s) has no matches.\n".formatted(" ", constraint.score().toShortString(),
185+
constraint.constraintRef().constraintName()));
185186
} else {
186-
summary.append("""
187-
%s: constraint (%s) has %s matches:
188-
""".formatted(constraint.score().toShortString(), constraint.constraintRef().constraintName(),
189-
matches.size()));
187+
summary.append(
188+
"%8s%s: constraint (%s) has %s matches:\n".formatted(" ", constraint.score().toShortString(),
189+
constraint.constraintRef().constraintName(), matches.size()));
190190
}
191191
matches.stream()
192192
.sorted(matchScoreComparator)
193193
.limit(DEFAULT_SUMMARY_CONSTRAINT_MATCH_LIMIT)
194-
.forEach(match -> summary.append("""
195-
%s: justified with (%s)
196-
""".formatted(match.score().toShortString(),
197-
match.justification())));
194+
.forEach(match -> summary
195+
.append("%12s%s: justified with (%s)\n".formatted(" ", match.score().toShortString(),
196+
match.justification())));
198197
if (matches.size() > DEFAULT_SUMMARY_CONSTRAINT_MATCH_LIMIT) {
199-
summary.append("""
200-
...
201-
""");
198+
summary.append("%12s%s\n".formatted(" ", "..."));
202199
}
203200
});
204201

205202
return summary.toString();
206203
}
204+
205+
public boolean isSolutionInitialized() {
206+
return score().isSolutionInitialized();
207+
}
208+
209+
@Override
210+
public String toString() {
211+
return "Score analysis of score %s with %d constraints.".formatted(score, constraintMap.size());
212+
}
207213
}

core/src/test/java/ai/timefold/solver/core/api/score/analysis/ScoreAnalysisTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ Explanation of score (0):
126126

127127
// Complete score analysis
128128
var summary = scoreAnalysis.summarize();
129-
assertThat(scoreAnalysis.getConstraintAnalysis(constraintPackage, constraintName1).matchCount()).isEqualTo(0);
129+
assertThat(scoreAnalysis.getConstraintAnalysis(constraintPackage, constraintName1).matchCount()).isZero();
130130
assertThat(summary)
131131
.isEqualTo("""
132132
Explanation of score (3init/0):

0 commit comments

Comments
 (0)