gnodet commented on code in PR #22303:
URL: https://github.com/apache/camel/pull/22303#discussion_r3005058293
##########
components/camel-mail/src/main/java/org/apache/camel/component/mail/MailConverters.java:
##########
@@ -85,11 +84,11 @@ public static String toString(Multipart multipart) throws
MessagingException, IO
for (int i = 0; i < size; i++) {
BodyPart part = multipart.getBodyPart(i);
Object content = part.getContent();
- while (content instanceof MimeMultipart) {
- if (multipart.getCount() < 1) {
+ while (content instanceof MimeMultipart mimeMultipart) {
+ if (mimeMultipart.getCount() < 1) {
Review Comment:
**Semantic change** — this is not a purely mechanical transformation.
The original code checked `multipart.getCount() < 1`, where `multipart` is
the **method parameter** (the outer `Multipart`). That check was actually dead
code: since we're inside `for (int i = 0; i < size; i++)` where `size =
multipart.getCount()`, the outer count is always ≥ 1 when this line is reached.
The new code checks `mimeMultipart.getCount() < 1`, which is the **inner**
`MimeMultipart` (the `content` variable). This is the correct thing to check —
it guards against calling `getBodyPart(0)` on an empty multipart on the next
line.
So this is actually a **bug fix** (replacing a dead guard with a meaningful
one), not a mechanical refactoring. Worth noting since the PR claims "no
behavioral changes". The fix itself is correct and an improvement.
##########
components/camel-cxf/camel-cxf-soap/src/main/java/org/apache/camel/component/cxf/jaxws/DefaultCxfBinding.java:
##########
@@ -1178,26 +1178,26 @@ protected static List<Source>
getPayloadBodyElements(Message message, Map<String
part = ((Holder<?>) part).value;
}
- if (part instanceof Source) {
+ if (part instanceof Source source) {
Element element = null;
- if (part instanceof DOMSource) {
- element = getFirstElement(((DOMSource) part).getNode());
+ if (part instanceof DOMSource domSource) {
+ element = getFirstElement(domSource.getNode());
}
if (element != null) {
addNamespace(element, nsMap);
answer.add(new DOMSource(element));
} else {
- answer.add((Source) part);
+ answer.add(source);
}
if (LOG.isTraceEnabled()) {
LOG.trace("Extract body element {}",
element == null ? "null" : getXMLString(element));
}
- } else if (part instanceof Element) {
- addNamespace((Element) part, nsMap);
- answer.add(new DOMSource((Element) part));
+ } else if (part instanceof Element element) {
Review Comment:
nit: The pattern variable `element` here reuses the same name as the local
variable `Element element = null` declared at line 1182 in the `if` branch
above. They're in mutually exclusive scopes so this compiles and works
correctly, but it's mildly confusing when reading the code. Consider using a
different name like `elem` to make it visually distinct.
Not blocking.
--
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]
