mxm commented on PR #15494:
URL: https://github.com/apache/iceberg/pull/15494#issuecomment-3984311105
> the switch statement are a bit inconsistent w.r.t {} clauses, unguarded
operations and return vs yield.
>
> it'd be good to have policy here, using your experience as the source of
that policy.
>
> what do you think is best? I'm thinking maybe "any case clause with side
effects is in a {} clause, and prefer `yield` over `return` for its
completeness and ability to say `int x = switch(y) { ... }. unsure about the
merits of not having {} wrappers around case clauses which only return data.
**Curly brackets**: I generally tried to remove the curly brackets where
possible. For readability it can still make sense to use curly brackets, e.g.
when there is a comment in the case block.
**Unguarded operations**: Those aren't possible anymore with switch
expressions. IMHO we shouldn't be using legacy switch statements anymore. See
below for the Checkstyle rule to enforce this.
**return vs yield**: The most obvious case for `yield` is when you're
assigning a variable in a case. That should be replaced by a `yield`. As for
return statement, I'm not 100% sure. One thing to consider is that we don't
enforce yielding in methods today, i.e. return statement can be used freely
anywhere in the the method.
> Interesting followup here: would legacy switch() be something checkstyle
would block? Given the fallthrough risks of the legacy statement, it does make
sense.
Yes, there is this check to get rid of the legacy switch:
https://checkstyle.sourceforge.io/checks/coding/useenhancedswitch.html
>Did you find any switches where the fall-through behaviour was actually
required?
Yes, there are a couple of instances. They were easy to convert though. The
basic pattern is:
```java
switch (myEnum) {
case A:
case B:
// do something
break;
case C:
// do something else
}
```
becomes
```java
switch (myEnum) {
case A, B:
// do something
case C:
// do something else
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
