-
Notifications
You must be signed in to change notification settings - Fork 164
feat: add a producer id to BestSolutionChangedEvent #1913
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
triceo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea on the event type! I think we can still improve on it.
core/src/main/java/ai/timefold/solver/core/api/solver/event/BestSolutionChangedEvent.java
Outdated
Show resolved
Hide resolved
...va/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhase.java
Outdated
Show resolved
Hide resolved
core/src/main/java/ai/timefold/solver/core/impl/phase/custom/DefaultCustomPhase.java
Outdated
Show resolved
Hide resolved
core/src/test/java/ai/timefold/solver/core/impl/solver/DefaultSolverTest.java
Show resolved
Hide resolved
core/src/main/java/ai/timefold/solver/core/api/solver/event/BestSolutionChangedEvent.java
Outdated
Show resolved
Hide resolved
triceo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll have to ask the platform guys about serialization of the events. That will give us some context here.
core/src/main/java/ai/timefold/solver/core/api/solver/event/BestSolutionChangedEvent.java
Outdated
Show resolved
Hide resolved
core/src/main/java/ai/timefold/solver/core/api/solver/event/EventProducerId.java
Outdated
Show resolved
Hide resolved
...va/ai/timefold/solver/core/impl/constructionheuristic/DefaultConstructionHeuristicPhase.java
Outdated
Show resolved
Hide resolved
A Problem Change does not trigger a new best solution event if it occurs before Construction Heuristic finishes.
triceo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're missing the identification on the first initialized solution consumer. After that, we're good.
core/src/main/java/ai/timefold/solver/core/impl/phase/PhaseType.java
Outdated
Show resolved
Hide resolved
core/src/main/java/ai/timefold/solver/core/impl/phase/PhaseType.java
Outdated
Show resolved
Hide resolved
…olution Having a consumer that accepts the solution was fragile, since whenever we wanted to add additional info, we would need to create an additional overload. Using events allows us to add new methods without affecting user code.
triceo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only one comment. As agreed, I'll fix it myself.
core/src/main/java/ai/timefold/solver/core/api/solver/SolverJobBuilder.java
Show resolved
Hide resolved
|



No description provided.