-
Notifications
You must be signed in to change notification settings - Fork 2.3k
refactor: Convert if-else chains to switch expressions (#17874) #18965
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
refactor: Convert if-else chains to switch expressions (#17874) #18965
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 PR refactors if-else chains that use instanceof checks into modern Java 17 pattern matching switch expressions, improving code readability and maintainability.
- Converts traditional
instanceofchecks with explicit casting to pattern matching syntax - Replaces long
if-elsechains with more concise and readableswitchexpressions - Modernizes exception handling patterns using switch expressions
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| SearchSortValuesAndFormats.java | Converts sort value type checking from if-else chain to switch expression |
| ValueSource.java | Refactors value wrapping logic to use pattern matching switch |
| NestedHelper.java | Modernizes query type checking methods with switch expressions |
| BulkItemResponse.java | Simplifies response type serialization using switch |
| DocWriteRequest.java | Converts document request serialization to switch pattern |
| SocketChannelContext.java | Updates exception handling to use switch expression |
| NioSelectorGroup.java | Modernizes exception type checking with pattern matching |
| LoggerMessageFormat.java | Converts array type checking to switch expression |
| BytesReference.java | Simplifies stream type checking with switch |
| ExceptionsHelper.java | Refactors exception handling methods to use switch expressions |
| Numbers.java | Converts number type checking methods to pattern matching switch |
server/src/main/java/org/opensearch/search/SearchSortValuesAndFormats.java
Outdated
Show resolved
Hide resolved
|
❌ Gradle check result for 59c35f7: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
Hello maintainers, I am reopening this pull request. I had briefly closed it because I was surprised by the automated comment from the bot and wasn't sure what the next step was. 😅 I've reopened it now to proceed. If there's a specific action required from me based on the bot's message, could you please guide me? Thank you for your understanding. |
|
I've never seen that before. Did Copilot just perform a review unprompted? |
Signed-off-by: BeomSeogKim <[email protected]>
Signed-off-by: BeomSeogKim <[email protected]>
Signed-off-by: BeomSeogKim <[email protected]>
Signed-off-by: BeomSeogKim <[email protected]>
|
❌ Gradle check result for f384f62: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: BeomSeogKim <[email protected]>
f384f62 to
55675d6
Compare
|
❌ Gradle check result for 55675d6: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
This PR is stalled because it has been open for 30 days with no activity. |
58f7968 to
1fe001d
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #18965 +/- ##
============================================
+ Coverage 72.79% 72.89% +0.10%
- Complexity 69605 69630 +25
============================================
Files 5658 5658
Lines 320079 320061 -18
Branches 46345 46290 -55
============================================
+ Hits 232996 233307 +311
+ Misses 68230 67866 -364
- Partials 18853 18888 +35 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@andrross could you check one more time? I refactor the code causing test failure. |
sandeshkr419
left a comment
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 changes look neat with a small nit-pick.
Thanks for working on these changes.
Signed-off-by: BeomSeogKim <[email protected]>
… (opensearch-project#18965) Signed-off-by: BeomSeogKim <[email protected]>
… (opensearch-project#18965) Signed-off-by: BeomSeogKim <[email protected]> Signed-off-by: Ankit Jain <[email protected]>
… (opensearch-project#18965) Signed-off-by: BeomSeogKim <[email protected]> Signed-off-by: Ankit Jain <[email protected]>
… (opensearch-project#18965) Signed-off-by: BeomSeogKim <[email protected]>
… (opensearch-project#18965) Signed-off-by: BeomSeogKim <[email protected]>
… (opensearch-project#18965) Signed-off-by: BeomSeogKim <[email protected]>
Description
[Describe what this change achieves]
As proposed in issue #17874, this PR refactors several
if-elsechains that useinstanceofchecks into more readable and modern Java 17 pattern matchingswitchstatements. This improves code clarity and maintainability.Affected files:
- Numbers.java: toLongExact, toBigIntegerExact methods
- ExceptionsHelper.java: exception handling methods
- NestedHelper.java: query matching methods
- ValueSource.java: value wrapping logic
- SearchSortValuesAndFormats.java: sort value formatting
- BulkItemResponse.java: response type serialization
- DocWriteRequest.java: request serialization methods
Related Issues
Resolves #17874
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.