spring-projects / spring-data-jpa

Simplifies the development of creating a JPA-based data access layer.
https://spring.io/projects/spring-data-jpa/
Apache License 2.0
2.93k stars 1.39k forks source link

Fixed Unnecessary Computation Issue Caused by `String.format()` #3444

Closed dukbong closed 2 months ago

dukbong commented 2 months ago

This PR improved the code by using Assert methods instead of throwing IllegalArgumentException in if-else statements.

dukbong commented 2 months ago

Excessive switching from if statements that throw an exception into Assert utility usage is counterintuitive in many places. There are also occurrences that change the thrown exception type.

It makes sense to revisit Assert usages in combination with String.format to defer message creation. However, in places that explicitly throw exceptions, we would like to keep the code as-is. Once an exception is being thrown, we need to construct the message and that is a fine arrangement already.

I apologize for negatively impacting the open-source project management. I will close this PR and resubmit it with only the necessary operations from commit 33eb67a.

Before sending the PR, I will ensure that it is thoroughly checked multiple times.

I'm sorry again for the inconvenience caused.

dukbong commented 2 months ago

Excessive switching from if statements that throw an exception into Assert utility usage is counterintuitive in many places. There are also occurrences that change the thrown exception type. It makes sense to revisit Assert usages in combination with String.format to defer message creation. However, in places that explicitly throw exceptions, we would like to keep the code as-is. Once an exception is being thrown, we need to construct the message and that is a fine arrangement already.

I apologize for negatively impacting the open-source project management. I will close this PR and resubmit it with only the necessary operations from commit 33eb67a.

Before sending the PR, I will ensure that it is thoroughly checked multiple times.

I'm sorry again for the inconvenience caused.

3445

mp911de commented 2 months ago

First of all, do not worry, and there's no need to apologize. We're happy to provide guidance.

Please also note that you can force-push into a pull request until the final merge so that you can reuse already open pull requests.

dukbong commented 2 months ago

Oh, I wasn't aware of that. I have now created a separate PR for the parts that urgently need to be fixed. I'll keep this in mind and use it more effectively going forward.