-
Notifications
You must be signed in to change notification settings - Fork 170
feat: introduce flatten() as a more generic alternative to flattenLast() #2018
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
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.
Pull request overview
This pull request introduces a new flatten() method as a more generic alternative to the existing flattenLast() method in the constraint streams API. The key difference is that flatten() preserves all original tuple elements and appends new elements from the iterable, while flattenLast() replaces the last element with items from the iterable.
Changes:
- Added
flatten()methods to UniConstraintStream, BiConstraintStream, and TriConstraintStream APIs - Implemented corresponding Bavet constraint stream classes and node classes for the new flatten operation
- Updated documentation to explain both
flatten()andflattenLast(), positioningflattenLast()as a simplified version - Refactored AbstractFlattenLastNode to remove one generic type parameter and change the abstract method signature
- Added test coverage for the new flatten() operation across Uni, Bi, and Tri constraint streams
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| score-calculation.adoc | Updated documentation to introduce flatten() first, then explain flattenLast() as a simplified alternative |
| UniConstraintStream.java | Added flatten() API method with BiConstraintStream return type, updated flattenLast() javadoc |
| BiConstraintStream.java | Added flatten() API method with TriConstraintStream return type, updated flatten/flattenLast javadoc |
| TriConstraintStream.java | Added flatten() API method with QuadConstraintStream return type |
| BavetFlattenUniConstraintStream.java | New implementation class for UniConstraintStream.flatten() |
| BavetFlattenBiConstraintStream.java | New implementation class for BiConstraintStream.flatten() |
| BavetFlattenTriConstraintStream.java | New implementation class for TriConstraintStream.flatten() |
| BavetAbstractUniConstraintStream.java | Added flatten() method implementation |
| BavetAbstractBiConstraintStream.java | Added flatten() method implementation |
| BavetAbstractTriConstraintStream.java | Added flatten() method implementation |
| BavetFlattenLastUniConstraintStream.java | Refactored to inline variables and use var |
| BavetFlattenLastBiConstraintStream.java | Removed unused imports, refactored to inline variables and use var |
| BavetFlattenLastTriConstraintStream.java | Refactored to inline variables |
| BavetFlattenLastQuadConstraintStream.java | Refactored to inline variables |
| FlattenUniNode.java | New node implementation for UniConstraintStream.flatten() |
| FlattenBiNode.java | New node implementation for BiConstraintStream.flatten() |
| FlattenTriNode.java | New node implementation for TriConstraintStream.flatten() |
| FlattenLastUniNode.java | Updated to match refactored AbstractFlattenLastNode |
| FlattenLastBiNode.java | Updated to match refactored AbstractFlattenLastNode |
| FlattenLastTriNode.java | Updated to match refactored AbstractFlattenLastNode |
| FlattenLastQuadNode.java | Updated to match refactored AbstractFlattenLastNode |
| AbstractFlattenLastNode.java | Refactored to remove one generic parameter and change abstract method from getEffectiveFactIn to extractIterable |
| FlattenLastUniNodeTest.java | Updated generic type parameters in test |
| AbstractUniConstraintStreamTest.java | Added flatten() test |
| AbstractBiConstraintStreamTest.java | Added flatten() test |
| AbstractTriConstraintStreamTest.java | Added flatten() test |
| ConstraintStreamFunctionalTest.java | Added default flatten() method to test interface |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ava/ai/timefold/solver/core/impl/score/stream/bavet/tri/BavetFlattenTriConstraintStream.java
Outdated
Show resolved
Hide resolved
.../java/ai/timefold/solver/core/impl/score/stream/bavet/bi/BavetFlattenBiConstraintStream.java
Outdated
Show resolved
Hide resolved
core/src/main/java/ai/timefold/solver/core/api/score/stream/bi/BiConstraintStream.java
Outdated
Show resolved
Hide resolved
core/src/main/java/ai/timefold/solver/core/api/score/stream/uni/UniConstraintStream.java
Outdated
Show resolved
Hide resolved
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.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
core/src/main/java/ai/timefold/solver/core/impl/bavet/common/AbstractFlattenNode.java:21
- The field name
flattenLastStoreIndexis misleading since this class is now used for bothflatten()andflattenLast()operations. Consider renaming it to something more generic likeflattenStoreIndexortupleStoreIndexto better reflect its purpose.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
core/src/main/java/ai/timefold/solver/core/impl/bavet/uni/FlattenUniNode.java
Outdated
Show resolved
Hide resolved
core/src/main/java/ai/timefold/solver/core/impl/bavet/bi/FlattenBiNode.java
Outdated
Show resolved
Hide resolved
core/src/main/java/ai/timefold/solver/core/impl/bavet/tri/FlattenTriNode.java
Outdated
Show resolved
Hide resolved
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.
The code looks good to me; you might want to add flatten variants to
Lines 198 to 202 in 02e2c8b
| void differentParentSameFunctionFlattenLast(); | |
| void sameParentDifferentFunctionFlattenLast(); | |
| void sameParentSameFunctionFlattenLast(); |
and
Lines 24 to 26 in 02e2c8b
| void flattenLast(); | |
| void flattenLastNewInstances(); |
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.
Pull request overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ava/ai/timefold/solver/core/impl/score/stream/bavet/tri/BavetFlattenTriConstraintStream.java
Outdated
Show resolved
Hide resolved
|



No description provided.