Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use String.repeat() and pattern matching instanceof #27834

Closed
wants to merge 1 commit into from
Closed

Use String.repeat() and pattern matching instanceof #27834

wants to merge 1 commit into from

Conversation

slutmaker
Copy link
Contributor

@slutmaker slutmaker commented Dec 17, 2021 •
edited by sbrannen
Loading

Using various new features from Java 9-17 such as String.repeat() and pattern matching instanceof.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 17, 2021
@slutmaker slutmaker force-pushed the use_new_java_version_features branch from 34f0f99 to f728ef7 Compare December 17, 2021 18:48
JarvisCraft
JarvisCraft suggested changes Dec 17, 2021
View reviewed changes
...-jdbc/src/main/java/org/springframework/jdbc/support/SQLErrorCodeSQLExceptionTranslator.java Outdated
@@ -360,38 +360,44 @@ protected DataAccessException createCustomException(
// invoke constructor
Constructor<?> exceptionConstructor;
switch (constructorType) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess that should be replaced with single return switch with yields inside.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

.../java/org/springframework/jdbc/support/incrementer/AbstractDataFieldMaxValueIncrementer.java
}
sb.append(s);
s = sb.toString();
s = "0".repeat(this.paddingLength - len) + s;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be less efficient due to the need for allocation of a temporary String.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JMH says otherwise
изображение

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider attaching benchmark's code

@sbrannen
Copy link
Member

@AbstractCoderX, have you run a full local build?

./gradlew check

Just curious, because the last time I checked.... we could not use switch expressions due to a limitation with Checkstyle (specifically Spring Java Format).

@sbrannen sbrannen added status: waiting-for-feedback We need additional information before we can continue type: task A general task labels Dec 19, 2021
@slutmaker
Copy link
Contributor Author

Just curious, because the last time I checked.... we could not use switch expressions due to a limitation with Checkstyle (specifically Spring Java Format).

As far as I can see, this has already been fixed, why does the codestyle check fail?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 23, 2021
@sbrannen
Copy link
Member

sbrannen commented Jan 3, 2022

Just curious, because the last time I checked.... we could not use switch expressions due to a limitation with Checkstyle (specifically Spring Java Format).

As far as I can see, this has already been fixed, why does the codestyle check fail?

The fix is in spring-javaformat 0.0.30 which has not been released. We are therefore still using spring-javaformat 0.0.29.

spring-framework/build.gradle

Line 323 in b24cbc5

checkstyle("io.spring.javaformat:spring-javaformat-checkstyle:0.0.29")

@sbrannen
Copy link
Member

sbrannen commented Jan 3, 2022

The fix is in spring-javaformat 0.0.30 which has not been released. We are therefore still using spring-javaformat 0.0.29.

spring-framework/build.gradle

Line 323 in b24cbc5

checkstyle("io.spring.javaformat:spring-javaformat-checkstyle:0.0.29")

In light of that, @AbstractCoderX feel free to move the "switch expression" changes to a separate PR so that we can merge in the other changes independent of that.

@sbrannen sbrannen added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jan 3, 2022
@slutmaker slutmaker force-pushed the use_new_java_version_features branch from 6eb6f4a to 727b14b Compare January 3, 2022 14:57
@slutmaker
Copy link
Contributor Author

The fix is in spring-javaformat 0.0.30 which has not been released. We are therefore still using spring-javaformat 0.0.29.

spring-framework/build.gradle

Line 323 in b24cbc5

checkstyle("io.spring.javaformat:spring-javaformat-checkstyle:0.0.29")

In light of that, @AbstractCoderX feel free to move the "switch expression" changes to a separate PR so that we can merge in the other changes independent of that.

done

@sbrannen sbrannen changed the title Use new java version features Use String.repeat() and pattern matching instanceof Jan 3, 2022
@sbrannen sbrannen self-assigned this Jan 3, 2022
@sbrannen sbrannen removed status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 3, 2022
@sbrannen sbrannen added this to the 6.0.0-M2 milestone Jan 3, 2022
@sbrannen sbrannen closed this in ab240f5 Jan 3, 2022
@sbrannen
Copy link
Member

sbrannen commented Feb 1, 2022

@AbstractCoderX, are you planning on submitting a PR with the "switch expression" changes?

@slutmaker
Copy link
Contributor Author

@AbstractCoderX, are you planning on submitting a PR with the "switch expression" changes?

I will do it this week

@slutmaker
Copy link
Contributor Author

@sbrannen #28014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JarvisCraft JarvisCraft JarvisCraft requested changes

Assignees

@sbrannen sbrannen

Labels
type: task A general task
Projects
None yet
Milestone
6.0.0-M2
Development

Successfully merging this pull request may close these issues.
4 participants
@slutmaker @sbrannen @JarvisCraft @spring-projects-issues