Submission: switch (...) instanceof feature

[Derek Foster]

Like Gabriel, I have severe reservations about this proposal. I am concerned that it will encourage people to avoid the common OOP "best practice" of using virtual method dispatch in favor of doing an explicit "switch" on each subtype of a type, in cases where doing so is not necessary.

I concede that there are a few (FEW!) places where doing the equivalent of a switch on instances of a type is necessary (for instance, if the type is in a library you don't have the ability to change). I can see some value in this proposal for that purpose. However, I would very much not like to see use of this construct become commonplace. I already see too many instances of people doing the equivalent of "if (object.type == FOO) {doThis(); } else if (object.type == BAR) { doThat(); }" instead of writing "object.doSomething();"

Derek

-----Original Message-----
>From: Gabriel Belingueres <belingueres at gmail.com>
>Sent: Mar 30, 2009 7:31 AM
>To: Jeroen van Maanen <jeroen at entreact.com>
>Cc: coin-dev at openjdk.java.net
>Subject: Re: Submission: switch (...) instanceof feature
>
>IMO I'm against this.
>
>First, it is against current best practices for the design of
>object-oriented software to make easier to code something with a case
>statement on types/classes.
>
>Second:
> void log(Object object) {
>   switch (object) instanceof {
>   case String:
>     logger.debug("'" + object + "'");
>   case Date:
>     logger.debug(object.getTime());
>   case void:
>     logger.debug("null");
>   default:
>     logger.debug("<" + object.toString() + ">");
>   }
> }
>
>It think it is clearer (when possible) writing it with several
>overloaded methods and double dispatching.
>
>Third:
>   } catch (Exception exception) {
>     switch (exception.getCause()) instanceof {
>     case ParseException:
>       log.warn("Could not get status for '" + id + ": " +
>exception.getCause());
>     default:
>       log.warn("Could not get status for '" + id + ", exception);
>     }
>}
>
>in the case you intentionally left out the break statement, then the
>switch statement is not any clearer than doing an if.
>in the case that you wanted the break statement on the ParseException
>case, it is even clearer to use two catch blocks (one for
>ParseException and other for Throwable.
>
>
>2009/3/29 Jeroen van Maanen <jeroen at entreact.com>:
>> I'd like to coin a switch (...) instanceof statement as a new feature of the
>> Java language. Please accept the attached proposal for review.
>>
>> Regards, Jeroen
>>
>> PROJECT COIN SMALL LANGUAGE CHANGE PROPOSAL FORM v1.0
>>
>> AUTHOR(S): Jeroen van Maanen <http://lexau.org/contact.html>
>>
>> OVERVIEW
>>
>> FEATURE SUMMARY: The instanceof switch statement allows for clear and
>> concise
>> handling of alternatives that depend on the type of a given object.
>>
>> MAJOR ADVANTAGE: The instanceof switch statement removes the need for
>> different
>> names for the same object with different types and the extra declarations
>> and
>> casts to define those names.
>>
>> MAJOR BENEFIT: Why is the platform better if the proposal is adopted?
>>
>> MAJOR DISADVANTAGE: Coders, reviewers, and IDE's need to be able to read the
>> new statement and interpret and treat it correclty.
>>
>> ALTERNATIVES: There are no alternatives.
>>
>> EXAMPLES
>>
>> SIMPLE EXAMPLE:
>>
>>  void log(Object object) {
>>    switch (object) instanceof {
>>    case String:
>>      logger.debug("'" + object + "'");
>>    case Date:
>>      logger.debug(object.getTime());
>>    case void:
>>      logger.debug("null");
>>    default:
>>      logger.debug("<" + object.toString() + ">");
>>    }
>>  }
>>
>> ADVANCED EXAMPLE:
>>
>>  public StatusEnum getStatus(String id) {
>>    StatusEnum result;
>>    try {
>>      result = internalGetStatus(id);
>>    } catch (Exception exception) {
>>      switch (exception.getCause()) instanceof {
>>      case ParseException:
>>        log.warn("Could not get status for '" + id + ": " +
>> exception.getCause());
>>      default:
>>        log.warn("Could not get status for '" + id + ", exception);
>>      }
>>    }
>>  }
>>
>> public class PrettyPrinter {
>>  private Writer writer;
>>
>>  public PrettyPrinter(Writer writer) {
>>    this.writer = writer;
>>  }
>>
>>  public write(Object object) {
>>    switch (object) instanceof {
>>      case String:
>>        writer.write(stringDenotation(object));
>>      case Collection:
>>        writer.write(object.getClass().getSimpleName() + ": [");
>>        for (Object element : object) {
>>          write(element);
>>          writer.write(",");
>>        }
>>        writer.write("]");
>>      case Map:
>>        write(object.entrySet());
>>      case Map.Entry:
>>        write(object.getKey());
>>        writer.write(":");
>>        write(object.getValue());
>>      case void:
>>        writer.write("null");
>>      default:
>>        // TODO: deal with arrays of unknown base type
>>        writer.write("<" + object.toString() + ">");
>>    }
>>  }
>>
>>  private stringDenotation(String str) {
>>    ...
>>  }
>>
>> }
>>
>> DETAILS
>>
>> SPECIFICATION: The switch instanceof feature adds an alternative to the
>> switch
>> statement to the grammar.
>>
>>  SwitchStatement:
>>    switch ( Expression ) SwitchBlock
>>    switch ( Identifier ) instanceof TypeSwitchBlock
>>
>>  TypeSwitchBlock:
>>    { TypeSwitchBlockStatementGroups? TypeSwitchLabels? }
>>
>>  TypeSwitchBlockStatementGroups:
>>    TypeSwitchBlockStatementGroup
>>    TypeSwitchBlockStatementGroups TypeSwitchBlockStatementGroup
>>
>>  TypeSwitchBlockStatementGroup:
>>    TypeSwitchLabel BlockStatements
>>
>>  TypeSwitchLabel:
>>    case Type :
>>    case void :
>>    default :
>>
>> COMPILATION: The statement
>>
>>  switch (<<identifier>>) instanceof {
>>  case <<type1>>:
>>    <<statements1 using identifier>>
>>  case <<type2>>:
>>    <<statements2>>
>>  ...
>>  case void:
>>    <<statementsVoid>>
>>  ...
>>  default:
>>    <<statementsDefault>>
>>  }
>>
>> Would be compiled by desugaring it to
>>
>>  if (<<identifier>> instanceof <<type1>>) {
>>    <<type1>> <<identifier + "$1">> = (<<type1>>) <<identifier>>;
>>    <<statements1 with identifier replaced by identifier$1>>
>>  }
>>  else if (<<identifier>> instanceof <<type2>>) {
>>    <<type2>> <<identifier + "$2">> = (<<type2>>) <<identifier>>;
>>    <<statements2 with identifier replaced by identifier + "$2">>
>>  }
>>  ...
>>  else if (<<identifier>> == null) {
>>    <<statementsVoid>>
>>  }
>>  ...
>>  else {
>>    <<statementsDefault>>
>>  }
>>
>> TESTING: The feature can be tested by compiling and running the examples and
>> comparing the results with the had coded expected desugared versions.
>>
>> LIBRARY SUPPORT: The feature needs no additional library support.
>>
>> REFLECTIVE APIS: The feature does not affect reflective api's.
>>
>> OTHER CHANGES: The feature does not need other changes.
>>
>> MIGRATION: This feature does not invalidate existing code. An existing code
>> base could be scanned for type casts. Occurrences of type casts should be
>> manually evaluated and, if desired, recoded using the new feature.
>>
>> COMPATIBILITY
>>
>> BREAKING CHANGES: This feature does not invalidate existing code.
>>
>> EXISTING PROGRAMS: This feature has no impact on existing code.
>>
>> REFERENCES
>>
>> EXISTING BUGS: There are no existing Sun bug ids related to this proposal.
>>
>> URL FOR PROTOTYPE: Not available.
>>
>>
>>
>>
>