remove unnecessary variable assignments #1070
Conversation
|
This PR needs Approvals as follows.
Please choose reviewers and requet reviews! Click to see how to approve each reviewsYou can approve this PR by triggered comments as follows.
See all trigger commentsPlease replace [Target] to review target
|
|
To reduce the amount of work on readability reviewers, please send readability review request after you get LGTM and Ownership Approval. |
OK, I understand. |
| checked_val = lower | ||
| return checked_val | ||
| def clamp(lower, upper, target): | ||
| if target < lower: |
There was a problem hiding this comment.
How about
tmp = max(lower, target)
tmp = min(max, target)
return tmp
There was a problem hiding this comment.
What is the advantage of this?
There was a problem hiding this comment.
less lines and little bit readable.
Umm, but this is my personal opinion.
Not necessary change.
There was a problem hiding this comment.
How about return min(upper, max(lower, target)) ?
There was a problem hiding this comment.
@tfujiwar How do you think about this?
There was a problem hiding this comment.
How about return min(upper, max(lower, target))
I think that's the best way.
In general, reducing conditional branches helps to keep code simpler.
| elif lower > checked_val: | ||
| checked_val = lower | ||
| return checked_val | ||
| def clamp(lower, upper, target): |
There was a problem hiding this comment.
clamp(target, lower, upper) might be more straightforward.
|
noshiro seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
1 similar comment
|
noshiro seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
@lm-noshiro Can you check review comment? |
What this patch does to fix the issue.
Link to any relevant issues or pull requests.
#1058