chore: make score analysis/recommendations/update fail fast on inconsistent solution#2444
Conversation
8d712fc to
3bade17
Compare
3bade17 to
af16cb2
Compare
triceo
left a comment
triceo
left a comment
There was a problem hiding this comment.
Overall LGTM, with some comments.
We should also clearly document the cases in which inconsistency is going to result in runtime failures, but that may wait for a later PR.
| // Do nothing | ||
| } | ||
|
|
||
| public List<Object> getInconsistentEntities() { |
There was a problem hiding this comment.
I think a more fitting name would be computeInconsistentEntities to make it clear that this is expensive.
| lastVariableUpdateWasSuccessful = variableListenerSupport.triggerVariableListenersInNotificationQueues(); | ||
| } | ||
|
|
||
| public boolean isLastVariableUpdateWasSuccessful() { |
There was a problem hiding this comment.
IMO works equally as well, even better, without the "was" there. lastVariableUpdateSuccessful perhaps best.
| throw new IllegalArgumentException("Can not call " + this.getClass().getSimpleName() | ||
| + ".update() with this solutionUpdatePolicy (" + solutionUpdatePolicy + ")."); | ||
| throw new IllegalArgumentException( | ||
| "Can not call %s.update() with this solutionUpdatePolicy (%s)." |
There was a problem hiding this comment.
I know this is not actually new, but maybe we add a hint as to why they cannot call this? Like this, the exception message is not very helpful.
| TimefoldSolverEnterpriseService.loadOrFail(TimefoldSolverEnterpriseService.Feature.SCORE_ANALYSIS); | ||
| var currentScore = (Score_) scoreDirectorFactory.getSolutionDescriptor().getScore(solution); | ||
| var analysis = callScoreDirector(solution, solutionUpdatePolicy, | ||
| var analysis = callScoreDirector("Solution analysis", solution, solutionUpdatePolicy, |
There was a problem hiding this comment.
I think we call it "Score analysis", don't we?
2 participants
Depends on #2443
Fail fast on inconsistent solution for
SolutionManager'supdateandanalyzemethods.