<div dir="ltr"><div>Hello!</div><div><br></div><div>Does not sound convincing. First, to my experience, it's quite widely applicable. My first two samples were from what you called a low-level libraries just because I wanted to grep something well-known. Now I grepped jdk10 source by `\w+ == \w+\.class` and scanned manually about 10% of the results and found about 10 places where it's useful (some examples are shown below). So extrapolating I may assume that this construct can be applied roughly 100 times in JDK (note that my regexp does not cover xyz.equals(foo.class) and some developers prefer this style; also different spacing is not covered). You may surely call the JDK code as "low-level libraries", but grepping IntelliJ IDEA source code I also see significant amount of occurrences. Though I don't see why usefulness of the feature in a low-level libraries should be the warning sign. </div><div><br></div><div>In any case I'm pretty sure that switch on class will be more applicable, than the switch on floats. But you are doing
<span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">the<span> </span></span>switch on floats. Why? For consistency, of course. You want to support all literals in switch. But class literals are also literals, according to JLS 15.8.2, so it is inconsistent not to support them (especially taking into account that their usefulness is not the lowest of all possible literals). Another comparison: all literals (including class literals) and enum values are acceptable as annotation values. The same in switch expressions, but excluding the class literals, which is inconsistent.</div><div><br></div><div>I don't buy an error-prone argument either. Is `switch(doubleValue) {case Math.PI: ...}` error-prone? Why somebody cannot assume that the comparison should tolerate some delta difference between doubleValue and Math.PI? Somebody surely can, but that's silly. We know that the switch checks for equality, it was always so. It will be so for classes as well, and assuming something different is inconsistent. After all, writing foo.equals(Bar.class) or foo == Bar.class is allowed in the language, people use these constructions, and often it's the right thing to do. Of course their code becomes erroneous sometimes, because in this particular place the inheritance should be taken into account. But the same is true for doubleValue == Math.PI comparison: sometimes it's ok, sometimes it's wrong and some tolerance interval should be checked instead. And when it's ok, you add a new option to use switch on doubles.</div><div><br></div><div>Several code samples found in JDK:</div><div><br></div><div>1. javafx.base/javafx/util/converter/LocalDateTimeStringConverter.java:197 (final classes)</div><div>if (type == LocalDate.class) {</div><div> return (T)LocalDate.from(chronology.date(temporal));</div><div>} else if (type == LocalTime.class) {</div><div> return (T)LocalTime.from(temporal);</div><div>} else {</div><div> return (T)LocalDateTime.from(chronology.localDateTime(temporal));</div><div>}</div><div><br></div><div>2. java.desktop/sun/print/Win32PrintService.java:928 (final classes)</div><div><div>    if (category == ColorSupported.class) {</div><div>      int caps = getPrinterCapabilities();</div><div>      if ((caps & DEVCAP_COLOR) != 0) {</div><div>        return (T)ColorSupported.SUPPORTED;</div><div>      } else {</div><div>        return (T)ColorSupported.NOT_SUPPORTED;</div><div>      }</div><div>    } else if (category == PrinterName.class) {</div><div>      return (T)getPrinterName();</div><div>    } else if (category == PrinterState.class) {</div><div>      return (T)getPrinterState();</div><div>    } else if (category == PrinterStateReasons.class) {</div><div>      return (T)getPrinterStateReasons();</div><div>    } else if (category == QueuedJobCount.class) {</div><div>      return (T)getQueuedJobCount();</div><div>    } else if (category == PrinterIsAcceptingJobs.class) {</div><div>      return (T)getPrinterIsAcceptingJobs();</div><div>    } else {</div><div>      return null;</div><div>    }</div></div><div>3. com.sun.media.sound.SoftSynthesizer#getPropertyInfo (line 926) - final classes; several blocks like this</div><div><div>              if (c == Byte.class)</div><div>                item2.value = Byte.valueOf(s);</div><div>              else if (c == Short.class)</div><div>                item2.value = Short.valueOf(s);</div><div>              else if (c == Integer.class)</div><div>                item2.value = Integer.valueOf(s);</div><div>              else if (c == Long.class)</div><div>                item2.value = Long.valueOf(s);</div><div>              else if (c == Float.class)</div><div>                item2.value = Float.valueOf(s);</div><div>              else if (c == Double.class)</div><div>                item2.value = Double.valueOf(s);</div></div><div>4. java.awt.Component#getListeners (interfaces!), Window#getListeners, List#getListeners, JComponent#getListeners, etc. are similar</div><div><div>  public <T extends EventListener> T[] getListeners(Class<T> listenerType) {</div><div>    EventListener l = null;</div><div>    if (listenerType == ComponentListener.class) {</div><div>      l = componentListener;</div><div>    } else if (listenerType == FocusListener.class) {</div><div>      l = focusListener;</div><div>    } else if (listenerType == HierarchyListener.class) {</div><div>      l = hierarchyListener;</div><div>    } else if (listenerType == HierarchyBoundsListener.class) {</div><div>      l = hierarchyBoundsListener;</div><div>    } else if (listenerType == KeyListener.class) {</div><div>      l = keyListener;</div><div>    } else if (listenerType == MouseListener.class) {</div><div>      l = mouseListener;</div><div>    } else if (listenerType == MouseMotionListener.class) {</div><div>      l = mouseMotionListener;</div><div>    } else if (listenerType == MouseWheelListener.class) {</div><div>      l = mouseWheelListener;</div><div>    } else if (listenerType == InputMethodListener.class) {</div><div>      l = inputMethodListener;</div><div>    } else if (listenerType == PropertyChangeListener.class) {</div><div>      return (T[])getPropertyChangeListeners();</div><div>    }</div><div>    return AWTEventMulticaster.getListeners(l, listenerType);</div><div>  }</div></div><div>5. java.beans.XMLEncoder#primitiveTypeFor (final classes)</div><div><div>    if (wrapper == Boolean.class) return Boolean.TYPE;</div><div>    if (wrapper == Byte.class) return Byte.TYPE;</div><div>    if (wrapper == Character.class) return Character.TYPE;</div><div>    if (wrapper == Short.class) return Short.TYPE;</div><div>    if (wrapper == Integer.class) return Integer.TYPE;</div><div>    if (wrapper == Long.class) return Long.TYPE;</div><div>    if (wrapper == Float.class) return Float.TYPE;</div><div>    if (wrapper == Double.class) return Double.TYPE;</div><div>    if (wrapper == Void.class) return Void.TYPE;</div><div>    return null;</div><div> 6. javax.swing.plaf.synth.SynthTableUI.SynthTableCellRenderer#configureValue (mix of abstract, non-final and final classes)</div></div><div><div>    private void configureValue(Object value, Class<?> columnClass) {</div><div>      if (columnClass == Object.class || columnClass == null) { // case Object.class, null!</div><div>        setHorizontalAlignment(JLabel.LEADING);</div><div>      } else if (columnClass == Float.class || columnClass == Double.class) {</div><div>        if (numberFormat == null) {</div><div>          numberFormat = NumberFormat.getInstance();</div><div>        }</div><div>        setHorizontalAlignment(JLabel.TRAILING);</div><div>        setText((value == null) ? "" : ((NumberFormat)numberFormat).format(value));</div><div>      }</div><div>      else if (columnClass == Number.class) {</div><div>        setHorizontalAlignment(JLabel.TRAILING);</div><div>        // Super will have set value.</div><div>      }</div><div>      else if (columnClass == Icon.class || columnClass == ImageIcon.class) {</div><div>        setHorizontalAlignment(JLabel.CENTER);</div><div>        setIcon((value instanceof Icon) ? (Icon)value : null);</div><div>        setText("");</div><div>      }</div><div>      else if (columnClass == Date.class) {</div><div>        if (dateFormat == null) {</div><div>          dateFormat = DateFormat.getDateInstance();</div><div>        }</div><div>        setHorizontalAlignment(JLabel.LEADING);</div><div>        setText((value == null) ? "" : ((Format)dateFormat).format(value));</div><div>      }</div><div>      else {</div><div>        configureValue(value, columnClass.getSuperclass()); // note this: recursively going to superclass automatically</div><div>      }</div><div>    }</div></div><div><br></div><div>With best regards,</div><div>Tagir Valeev.</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 9, 2018 at 8:38 PM, Brian Goetz <span dir="ltr"><<a href="mailto:brian.goetz@oracle.com" target="_blank">brian.goetz@oracle.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I'm skeptical of this feature, because (a) its not as widely applicable as it looks, (b) its error-prone.<br>
<br>
Both of these stem from the fact that comparing classes with == excludes subtypes. So it really only works with final classes -- but if we had a feature like this, people might mistakenly use it with nonfinal classes, and be surprised when a subtype shows up (this can happen even when your IDE tells you there are no subtypes, because of dynamic proxies). And all of the examples you show are in low-level libraries, which is a warning sign.<br>
<br>
Where did these snippets get their Class from? Good chance, case 1 got it from calling Object.getClass(). In which case, they can just pattern match on the type of the thing:<br>
<br>
   switch (date) {<br>
       case Date d: ...<br>
       case Timestamp t: ...<br>
       default: ...<br>
   }<br>
<br>
Case 2 is more likely just operating on types that it got from a reflection API. If you have only a few entries, an if-else will do; if you have more entries, a Map is likely to be the better choice. For situations like this, I'd rather invest in map literals or better Map.of() builders.<br>
<br>
So, I would worry this feature is unlikely to carry its weight, and further, may lead to misuse.<div class="HOEnZb"><div class="h5"><br>
<br>
<br>
On 4/9/2018 1:07 AM, Tagir Valeev wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hello!<br>
<br>
I don't remember whether switch on java.lang.Class instance was discussed. I guess, this pattern is quite common and it will be useful to support it. Such code often appears in deserialization logic when we branch on desired type to deserialize. Here are a couple of examples from opensource libraries:<br>
<br>
1. com.google.gson.DefaultDateTyp<wbr>eAdapter#read (gson-2.8.2):<br>
<br>
  Date date = deserializeToDate(in.nextStrin<wbr>g());<br>
  if (dateType == Date.class) {<br>
   return date;<br>
  } else if (dateType == Timestamp.class) {<br>
   return new Timestamp(date.getTime());<br>
  } else if (dateType == java.sql.Date.class) {<br>
   return new java.sql.Date(date.getTime());<br>
  } else {<br>
   // This must never happen: dateType is guarded in the primary constructor<br>
   throw new AssertionError();<br>
  }<br>
<br>
Could be rewritten as:<br>
<br>
  Date date = deserializeToDate(in.nextStrin<wbr>g());<br>
  return switch(dateType) {<br>
   case Date.class -> date;<br>
   case Timestamp.class -> new Timestamp(date.getTime());<br>
   case java.sql.Date.class -> new java.sql.Date(date.getTime());<br>
   default -><br>
    // This must never happen: dateType is guarded in the primary constructor<br>
    throw new AssertionError();<br>
  };<br>
<br>
2. com.fasterxml.jackson.databind<wbr>.deser.std.FromStringDeseriali<wbr>zer#findDeserializer (jackson-databind-2.9.4):<br>
<br>
  public static Std findDeserializer(Class<?> rawType)<br>
  {<br>
    int kind = 0;<br>
    if (rawType == File.class) {<br>
      kind = Std.STD_FILE;<br>
    } else if (rawType == URL.class) {<br>
      kind = Std.STD_URL;<br>
    } else if (rawType == URI.class) {<br>
      kind = Std.STD_URI;<br>
    } else if (rawType == Class.class) {<br>
      kind = Std.STD_CLASS;<br>
    } else if (rawType == JavaType.class) {<br>
      kind = Std.STD_JAVA_TYPE;<br>
    } else if // more branches like this<br>
    } else {<br>
      return null;<br>
    }<br>
    return new Std(rawType, kind);<br>
  }<br>
<br>
Could be rewritten as:<br>
<br>
  public static Std findDeserializer(Class<?> rawType)<br>
  {<br>
    int kind = switch(rawType) {<br>
    case File.class -> Std.STD_FILE;<br>
    case URL.class -> Std.STD_URL;<br>
    case URI.class -> Std.STD_URI;<br>
    case Class.cass -> Std.STD_CLASS;<br>
    case JavaType.class -> Std.STD_JAVA_TYPE;<br>
    ...<br>
    default -> 0;<br>
    };<br>
    return kind == 0 ? null : new Std(rawType, kind);<br>
  }<br>
<br>
In such code all branches are mutually exclusive. The bootstrap method can generate a lookupswitch based on Class.hashCode, then equals checks, pretty similar to String switch implementation. Unlike String hash codes Class.hashCode is not stable and varies between JVM launches, but they are already known during the bootstrap and we can trust them during the VM lifetime, so we can generate a lookupswitch. The minor problematic point is to support primitive classes like int.class. This cannot be passed directly as indy static argument, but this can be solved with condy.<br>
<br>
What do you think?<br>
<br>
With best regards,<br>
Tagir Valeev.<br>
<br>
</blockquote>
<br>
</div></div></blockquote></div><br></div>