On 28/04/2021 16:54, Brian Goetz wrote:
The think the I find mildly odd is that when Sandwich is added, it might not be enough to add another `case`. If there was null-related logic inside `case Box(Soup s):` that would have to be moved somewhere else. So while in most cases it will be a straightforward refactoring, I can see some puzzlers emerging here.

Can you pull on this string a big?


Let's say I start with this:


switch (lunch) {
        case Box(Soup s) {
             if (s == null) {
                  System.err.println("Box of null");
             } else {
                  System.err.println("Box of soup");
             }
        }

        case Bag(Soup s): {
             if (s == null) {
                  System.err.println("Bag of null");
             } else {
                  System.err.println("Bag of soup");
             }

        }

}


Then Sandwich is added to the hierarchy. The switch no longer compiles, I have to make it total. The following, which is, I believe, the first thing that will come to mind:

switch (lunch) {
        case Box(Soup s) {  /* same as before */ }
        case Box(Sandwich s) { ... }
        case Bag(Soup s): { /* same as before */ }
        case Bag(Sandwich s) { ... }
}


Now, question: is the above switch considered exhaustive? If not (because Box(null) is no longer matched), fine, the user will at least have some clue of what's happening.

But if it is considered exhaustive, then this will compile, but the null handling logic will be in the wrong place, and will be essentially dead code (which the user might be unaware of).

Assuming the above is not exhaustive, the user might then think to tweak to do this:

switch (lunch) {
        case Box(Soup s) {  /* same as before */ }
        case Box(Sandwich s) { ... }
        case Bag(Soup s): { /* same as before */ }
        case Bag(Sandwich s) { ... }
        default: ...
}

But that's again incorrect. We still have to move the null handling logic from `case Box(Soup)` and `case Box(Bar)` - so, the above code doesn't really cut it, since the `null` is nested, and not at the toplevel. So the better form would be this:

switch (lunch) {
        case Box(Soup s) { System.err.println("Box of soup"); }
        case Box(Sandwich s) { System.err.println("Box of sandwich"); }
        case Box(var x) { System.err.println("Box of null"); }
        case Bag(Soup s): { System.err.println("Bag of soup"); }
        case Bag(Sandwich s) { System.err.println("Bag of sandwich"); }
        case Bag(var x) { System.err.println("Bag of null"); }
}

I believe this is finally (1) total and (2) preserve the semantics of the original code. But it took quite a while to get here. And, in this new code, it seems like we're essentially using `case Box(var x)` (or `case Box(Object o)` it's the same) to really mean `case Box(null)`. Which is brittle - because, with this new code, if a new subtype is added (Pizza), the switch will remain total, no error would occur, but the could would behave incorrectly because what used to be a "catch-null" case has now turned into more than that.

Maurizio




Reply via email to