Use switch expressions introduced in Java 14 to simplify code#18371
Use switch expressions introduced in Java 14 to simplify code#18371ijuma wants to merge 1 commit intoapache:trunkfrom
Conversation
github-actions
Bot
added
streams
core
clolov
left a comment
clolov
left a comment
There was a problem hiding this comment.
I know this is marked as draft, but I had a short look and left a few minor comments
| case STRUCT: | ||
| return convertToStructInternal(toSchema, value); | ||
| } | ||
| throw new DataException("Unable to convert " + value + " (" + value.getClass() + ") to " + toSchema); |
There was a problem hiding this comment.
For my curiosity, will the exception here not change with the new syntax? If it changes, do upper layers handle it correctly?
There was a problem hiding this comment.
This case is never hit since the switch is exhaustive.
| case MAP: | ||
| return projectMap(source, record, target); | ||
| } | ||
| return null; |
There was a problem hiding this comment.
In other places you have translated this to default -> null in the new syntax. Shouldn't this be the same or am I missing something obvious?
There was a problem hiding this comment.
This was directly translated by IntelliJ - I started (but haven't completed) the review of every case to make sure it makes sense. Generally speaking, if the switch is exhaustive, then it doesn't need to return null if we're ok with throwing an exception if a new enum element is added later and the code is not updated.
| case DYNAMIC_METADATA_VOTER_DIRECTORY: | ||
| return "dynamic metadata voter directory"; | ||
| } | ||
| throw new RuntimeException("invalid enum type " + this); |
There was a problem hiding this comment.
Right - I'll confirm, but IntelliJ does this if its static analyzer decides the code is unreachable due to an exhaustive switch.
| case KRAFT_VERSION_1: | ||
| return (short) 1; | ||
| } | ||
| throw new IllegalStateException("Unsupported KRaft feature level: " + this); |
| case LEADER_EPOCH: | ||
| return FETCH_LEADER_EPOCH_CHECKPOINT; | ||
| } | ||
| return FETCH_SEGMENT; |
| case LEADER_EPOCH: | ||
| return LEADER_EPOCH_CHECKPOINT; | ||
| } | ||
| return SEGMENT; |
|
Seems we need to revert all KS related changed, as KS is still on Java 11. |
|
This PR is being marked as stale since it has not had any activity in 90 days. If you If you are having difficulty finding a reviewer, please reach out on the [mailing list](https://kafka.apache.org/contact). If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 30 days, it will be automatically closed. |
|
This PR has been closed since it has not had any activity in 120 days. If you feel like this |
4 participants
Need to fix some formatting issues before it's ready to review.
Committer Checklist (excluded from commit message)