pnowojski commented on a change in pull request #6974: [FLINK-10727][network]
remove unnecessary synchronization in SingleInputGate#requestPartitions()
URL: https://github.com/apache/flink/pull/6974#discussion_r229700439
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/consumer/SingleInputGate.java
##########
@@ -477,8 +477,8 @@ public boolean isFinished() {
@Override
public void requestPartitions() throws IOException,
InterruptedException {
- synchronized (requestLock) {
- if (!requestedPartitionsFlag) {
+ if (!requestedPartitionsFlag) {
Review comment:
1. yes, there is this general rule that having shorter/simpler branch first:
```
if (condition) {
one_liner
}
else {
sth
very
long
that
do
not
fit
in
programmer
brain's
cache
}
```
is better, since once reader gets to the else, he can already forget about
it and focus on the else branch, since there is "all of the remaining logic".
Putting it into extreme (with `zero_liner` else branch), your example:
```
if (condition) {
sth
very
long
that
while
reading
one
is
all
the
time
unsure
what
will
happen
in
else
branch
}
```
2. you are right, it doesn't matter.
% if/else branches order, LGTM
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
