Skip to content

[Feature Request] Make use of Java 17 pattern matching switch statements where possible #17874

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

Open
harshavamsi opened this issue Apr 10, 2025 · 8 comments · May be fixed by #17909
Open

[Feature Request] Make use of Java 17 pattern matching switch statements where possible #17874

harshavamsi opened this issue Apr 10, 2025 · 8 comments · May be fixed by #17909
Assignees
Labels
enhancement Enhancement or improvement to existing feature or request good first issue Good for newcomers

Comments

@harshavamsi
Copy link
Contributor

Is your feature request related to a problem? Please describe

Coming from #17769 (comment), there are many classes (and tests!) where we would replace multiple chained if-statements to make use of switch pattern matching to make them more readable

Describe the solution you'd like

An example class is NestedHelper.java where we match the query type in an if-else statement. This can be replaced to use switch statements

Related component

Other

Describe alternatives you've considered

No response

Additional context

No response

@harshavamsi harshavamsi added enhancement Enhancement or improvement to existing feature or request untriaged labels Apr 10, 2025
@github-actions github-actions bot added the Other label Apr 10, 2025
@harshavamsi harshavamsi added good first issue Good for newcomers and removed Other labels Apr 10, 2025
@JaySabva
Copy link

@harshavamsi i want to take on this issue.

JaySabva added a commit to JaySabva/OpenSearch that referenced this issue Apr 11, 2025
JaySabva added a commit to JaySabva/OpenSearch that referenced this issue Apr 11, 2025
@JaySabva JaySabva linked a pull request Apr 11, 2025 that will close this issue
3 tasks
@JaySabva
Copy link

@harshavamsi i have opened PR but dont merge it right now because i am going to make this kind of changes in other files.

JaySabva added a commit to JaySabva/OpenSearch that referenced this issue Apr 12, 2025
JaySabva added a commit to JaySabva/OpenSearch that referenced this issue Apr 12, 2025
JaySabva added a commit to JaySabva/OpenSearch that referenced this issue Apr 12, 2025
JaySabva added a commit to JaySabva/OpenSearch that referenced this issue Apr 12, 2025
JaySabva added a commit to JaySabva/OpenSearch that referenced this issue Apr 14, 2025
JaySabva added a commit to JaySabva/OpenSearch that referenced this issue Apr 14, 2025
JaySabva added a commit to JaySabva/OpenSearch that referenced this issue Apr 14, 2025
JaySabva added a commit to JaySabva/OpenSearch that referenced this issue Apr 14, 2025
JaySabva added a commit to JaySabva/OpenSearch that referenced this issue Apr 14, 2025
JaySabva added a commit to JaySabva/OpenSearch that referenced this issue Apr 14, 2025
JaySabva added a commit to JaySabva/OpenSearch that referenced this issue Apr 14, 2025
JaySabva added a commit to JaySabva/OpenSearch that referenced this issue Apr 14, 2025
JaySabva added a commit to JaySabva/OpenSearch that referenced this issue Apr 14, 2025
JaySabva added a commit to JaySabva/OpenSearch that referenced this issue Apr 14, 2025
JaySabva added a commit to JaySabva/OpenSearch that referenced this issue Apr 14, 2025
JaySabva added a commit to JaySabva/OpenSearch that referenced this issue Apr 14, 2025
@andrross
Copy link
Member

Catch All Triage - 1

@msfroh
Copy link
Contributor

msfroh commented Apr 28, 2025

@JaySabva -- Since this is just code cleanup, the existing unit + integration tests should be sufficient. We expect no changes in behavior, just friendlier code.

@JaySabva
Copy link

👍

@JaySabva
Copy link

@msfroh I might sound silly but I have one question regarding this #17909 (review)

How to clean up code like this? I don't see any good way to do it. Please if you can guide me.

@msfroh
Copy link
Contributor

msfroh commented Apr 28, 2025

@msfroh I might sound silly but I have one question regarding this #17909 (review)

How to clean up code like this? I don't see any good way to do it. Please if you can guide me.

Oh... I don't think that's possible in the general case.

For the Util.java case, here's the best I can come up with, but IMO it's uglier than the if/else statements:

    public static FileTree getJavaTestAndMainSourceResources(Project project, Action<? super PatternFilterable> filter) {
        final Optional<FileTree> testFileTree = getJavaTestSourceSet(project).map(SourceSet::getResources).map(FileTree::getAsFileTree);
        final Optional<FileTree> mainFileTree = getJavaMainSourceSet(project).map(SourceSet::getResources).map(FileTree::getAsFileTree);
        return switch (new boolean[]{ mainFileTree.isPresent(), testFileTree.isPresent()}) {
            case boolean[] arr when arr[0] && arr[1] -> testFileTree.get().plus(mainFileTree.get()).matching(filter);
            case boolean[] arr when arr[0] -> mainFileTree.get().matching(filter);
            case boolean[] arr when arr[1] -> testFileTree.get().matching(filter);
            default -> null;
        };
    }

@owaiskazi19 -- did you have something else in mind? I'm new to Java's pattern-matching in switch statements, so maybe there's a trick that I'm missing.

@owaiskazi19
Copy link
Member

@owaiskazi19 -- did you have something else in mind? I'm new to Java's pattern-matching in switch statements, so maybe there's a trick that I'm missing.

We could also use record pattern matching here

public static FileTree getJavaTestAndMainSourceResources(Project project, Action<? super PatternFilterable> filter) {
    var testFileTree = getJavaTestSourceSet(project).map(SourceSet::getResources).map(FileTree::getAsFileTree);
    var mainFileTree = getJavaMainSourceSet(project).map(SourceSet::getResources).map(FileTree::getAsFileTree);
    
    return switch (new Pair<>(testFileTree, mainFileTree)) {
        case Pair<Optional<FileTree>, Optional<FileTree>>(var test, var main) 
            when test.isPresent() && main.isPresent() -> 
            test.get().plus(main.get()).matching(filter);
        case Pair<Optional<FileTree>, Optional<FileTree>>(var test, var main) 
            when main.isPresent() -> 
            main.get().matching(filter);
        case Pair<Optional<FileTree>, Optional<FileTree>>(var test, var main) 
            when test.isPresent() -> 
            test.get().matching(filter);
        default -> null;
    };
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants