gnodet-bot commented on code in PR #27586:
URL: https://github.com/apache/camel/pull/27586#discussion_r4225411146
##########
components/camel-barcode/src/main/java/org/apache/camel/dataformat/barcode/BarcodeDataFormat.java:
##########
@@ -313,17 +314,21 @@ public final BarcodeParameters getParams() {
}
/**
- * The writer (encode) hint map.
+ * The writer (encode) hint map, as a read-only view. Use {@link
#addToHintMap(EncodeHintType, Object)} and
+ * {@link #removeFromHintMap(EncodeHintType)} to change it: the hints are
computed again when the data format
+ * starts, and only the changes made with these methods are applied again.
*/
public final Map<EncodeHintType, Object> getWriterHintMap() {
- return writerHintMap;
+ return Collections.unmodifiableMap(writerHintMap);
}
/**
- * The reader (decode) hint map.
+ * The reader (decode) hint map, as a read-only view. Use {@link
#addToHintMap(DecodeHintType, Object)} and
Review Comment:
💡 **Suggestion (minor):** Same as `getWriterHintMap` — the Javadoc documents
the read-only constraint but omits the live-view property. Adding "(live:
mutations via `addToHintMap`/`removeFromHintMap` are visible through this
view)" would make the contract self-contained.
##########
components/camel-barcode/src/test/java/org/apache/camel/dataformat/barcode/BarcodeDataFormatTest.java:
##########
@@ -229,6 +231,30 @@ final void testGetReaderHintMap() throws IOException {
}
}
+ /**
+ * A change made through the maps returned by the getters would bypass the
hints tracked by
+ * {@code addToHintMap}/{@code removeFromHintMap}, and be lost when the
data format starts again, so the maps are
+ * read-only.
+ */
+ @Test
+ final void testHintMapsAreReadOnly() throws IOException {
Review Comment:
💡 **Suggestion (minor):** The test verifies immutability (mutations throw),
but doesn't verify the *live-view* property — that a reference obtained before
a mutation via `addToHintMap` still reflects the change afterward.
`Collections.unmodifiableMap` is a live view, not a defensive copy, and that
distinction matters for callers who cache the returned map.
Consider adding:
```java
Map<EncodeHintType, Object> writerHints = instance.getWriterHintMap();
instance.addToHintMap(EncodeHintType.MARGIN, 5);
instance.start(); // recomputes hints
// writerHints (obtained before start) should now reflect the new margin
assertEquals(5, writerHints.get(EncodeHintType.MARGIN));
```
This would lock down the full contract (read-only *and* live) rather than
just the immutability side.
##########
components/camel-barcode/src/main/java/org/apache/camel/dataformat/barcode/BarcodeDataFormat.java:
##########
@@ -313,17 +314,21 @@ public final BarcodeParameters getParams() {
}
/**
- * The writer (encode) hint map.
+ * The writer (encode) hint map, as a read-only view. Use {@link
#addToHintMap(EncodeHintType, Object)} and
Review Comment:
💡 **Suggestion (minor):** The Javadoc says "read-only view" but doesn't
mention the *live* property — that the returned map reflects subsequent changes
made via `addToHintMap`/`removeFromHintMap`. This is a meaningful behavioral
distinction from a defensive copy. Consider adding a note like "(live:
mutations to the backing map via `addToHintMap`/`removeFromHintMap` are visible
through this view)" to make the contract explicit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
