-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
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: tidy up compiler #12874
base: main
Are you sure you want to change the base?
refactor: tidy up compiler #12874
Conversation
In general, I'm not a big fan of small refactorings like this when there are no obvious readability issues with the original code, which would make the history of the real logic changes harder to track. |
I think this will make the logical distinction more obvious. If it is not satisfied, it will return directly, and it will reduce the situation where the nesting level of many conditions is deep and the code readability is poor. |
I'm not saying that early return is not a useful pattern, and I use it myself. Rather, for code that has been around for a long time and without obvious readability issues, the benefits may not outweigh the costs. The costs include, but are not limited to, the cost of review by maintainers, the additional complexity of tracking changes to the code. |
Responses to previous efforts:
|
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
The PR fulfills these requirements:
main
branch for v2.x (or to a previous version branch)fix #xxx[,#xxx]
, where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information: