Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a copy constructor to the BiFunctionClassMap class to support creating independent copies of map instances. The change enables creating a new map with all mappings from an existing map while maintaining full independence.
Changes:
- Added a copy constructor to
BiFunctionClassMapthat creates a deep copy of the map and its internalClassMapper - Added comprehensive test coverage for the new copy constructor including edge cases, independence verification, and hierarchy resolution
- Updated GitHub Actions workflow to use
actions/checkout@v6
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| SpecsUtils/src/pt/up/fe/specs/util/classmap/BiFunctionClassMap.java | Adds copy constructor with covariant return type support, including deep copying of internal map and ClassMapper |
| SpecsUtils/test/pt/up/fe/specs/util/classmap/BiFunctionClassMapTest.java | Adds 8 comprehensive test cases covering copy constructor behavior, independence, hierarchy resolution, and edge cases |
| .github/workflows/nightly.yml | Updates checkout action from v4 to v6 |
SpecsUtils/src/pt/up/fe/specs/util/classmap/BiFunctionClassMap.java
Outdated
Show resolved
Hide resolved
lm-sousa
force-pushed
the
multi-weaver
branch
from
fbaacf7 to
543a0e1
Compare
| public <ER extends R> BiFunctionClassMap(BiFunctionClassMap<T, U, ER> other) { | ||
| this.map = new HashMap<>(); | ||
| for (var keyPair : other.map.entrySet()) { | ||
| this.map.put(keyPair.getKey(), (BiFunction<T, U, R>) keyPair.getValue()); | ||
| } | ||
| this.classMapper = new ClassMapper(other.classMapper); | ||
| } |
There was a problem hiding this comment.
The copy constructor is missing Javadoc documentation. Please add documentation that explains:
- The purpose of this constructor (creating a deep copy of another BiFunctionClassMap)
- The generic type parameter ER and its constraint (ER extends R, allowing covariant copying)
- The behavior regarding independence of the copy (changes to either map don't affect the other)
- Example usage if appropriate
This would align with the documentation style of other constructors and methods in the class.
| public <ER extends R> BiFunctionClassMap(BiFunctionClassMap<T, U, ER> other) { | ||
| this.map = new HashMap<>(); | ||
| for (var keyPair : other.map.entrySet()) { | ||
| this.map.put(keyPair.getKey(), (BiFunction<T, U, R>) keyPair.getValue()); |
There was a problem hiding this comment.
The unchecked cast on line 47 should be documented with a @SuppressWarnings annotation to clarify why it's safe. The cast is necessary because of Java's generic type system limitations with wildcards, but it's safe because ER extends R, ensuring type compatibility. Consider adding @SuppressWarnings("unchecked") to the constructor method signature, similar to how it's used in the get() methods in this class (lines 71 and 87).
No description provided.