break seen as a C archaism

[Kevin Bourrillion]

On Fri, Mar 9, 2018 at 3:21 PM, Louis Wasserman <lowasser at google.com> wrote:

Simplifying: let's call normal cases in a switch simple if they're a single
> statement or a no-op fallthrough, and let's call a default simple if it's a
> single statement or it's not there at all.
>
> Among switches apparently convertible to expression switches,
>
>    - 81% had all simple normal cases and a simple default.
>    - 5% had all simple normal cases and a nonsimple default.
>    - 12% had a nonsimple normal case and a simple default.
>    - 2% had a nonsimple normal case and a nonsimple default.
>
> I was surprised it was as high as 19%, so I grabbed a random sample of
these 45 occurrences from Google's codebase and reviewed them. My goal was
to find evidence that multi-statement cases in expression switches are
important and common. Spoiler: I found said evidence underwhelming.

There were 3 that I would call false matches (e.g. two that simply used a
void `return` instead of `break` after every case without reason).

There were fully 20 out of the remaining 42 that I quickly concluded should
be refactored regardless of anything else, and where that refactoring
happens to leave them with only simple cases and simple/no default. These
refactorings were varied (hoist out code common to all non-exception cases;
simplify unreachable code; change to `if` if only 1-2 cases; extract a
method (needing only 1-2 parameters) for a case that is much bigger than
the others; switch from loop to Streams; change `if/else` to ?:; move a
precondition check to a more appropriate location; and a few other varied
cleanups).

Next there were 7 examples where the non-simple cases included
side-effecting code, like setting fields or calling void methods. In Google
Style I expect that we will probably forbid (or at least strongly dissuade)
side effects in expression switch. I should probably bring this up
separately, but I am pretty convinced by now that users should see
expression switch and procedural switch as two completely different things,
and by convention should always keep the former purely functional.

Next there were 7 examples where a case was "non-simple" only because it
was using the "log, then return a null object (or null), instead of
throwing an exception" anti-pattern. I was surprised this was that popular.
and another 2 that used the "log-and-also-throw" anti-pattern.

2 examples had a use-once local variable that saved a *little* bit of
nesting. I wouldn't normally refactor these, but if expression switch had
no mechanism for multi-statement cases, I wouldn't think twice about it.

1 example had cases that looked nearly identical, 3 statements each, that
could all be hoisted out of the switch, except that the types that differed
across the three didn't implement a common interface (as they clearly
should have). Slightly compelling.

1 example had all simple cases except that one also wanted to check an
assertion. Okay, slightly compelling.

Finally, the cases that were the most compelling to me: 3 examples had one
or more large cases, where factoring them out into helper methods would
imho be ugly because >=3 parameters would be required. If expression switch
didn't permit multi-statement cases, I would just keep them as procedural
switches. It's only 3 out of 42.

Summary:

imho, early signs suggest that the grossness of `break x` is not *nearly*
justified by the actual observed positive value of supporting
multi-statement cases in expression switch. Are we open to killing that, or
would we be if I produced more and clearer evidence?





>
> On Fri, Mar 9, 2018 at 2:56 PM Brian Goetz <brian.goetz at oracle.com> wrote:
>
>> Did you happen to calculate what percentage was _not_ the "default"
>> case?  I would expect that to be a considerable fraction.
>>
>> On 3/9/2018 5:49 PM, Kevin Bourrillion wrote:
>>
>> On Fri, Mar 9, 2018 at 1:19 PM, Remi Forax <forax at univ-mlv.fr> wrote:
>>
>> When i asked what we should do instead, the answer is either:
>>>   1/ we should not allow block of codes in the expression switch but
>>> only expression
>>>   2/ that we should use the lambda syntax with return, even if the
>>> semantics is different from the lambda semantics.
>>>
>>> I do not like (1) because i think the expression switch will become
>>> useless
>>
>>
>> In our (large) codebase, +Louis determined that, among switch statements
>> that appear translatable to expression switch, 13.8% of them seem to
>> require at least one multi-statement case.
>>
>>
>>


-- 
Kevin Bourrillion | Java Librarian | Google, Inc. | kevinb at google.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/amber-spec-experts/attachments/20180313/e3f82b98/attachment.html>