Also, let's separate the problem from the solution here.
Problem: switching on class values.
Solution: make class literals be constant patterns.
I don't think the problem is unworthy of solution, but I don't like the
specific solution. But, there may be other ways to get there.
Here's two workarounds that you can do today:
switch (c.getName()) {
case "java.lang.String": ...
case "java.lang.Integer": ...
}
enum KnownTypes {
STRING(String.class), INTEGER(Integer.class), ...;
static Map<CLass, KnownTypes> classToEnum = new HashMap<>();
... constructor populates map ...
}
switch (KnownTypes.classToEnum.get(c)) {
case null: ...
case STRING: ...
case INTEGER: ...
}
These are both workarounds, for sure. (With map literals, we can make
either cleaner.)
What other approaches might there be? Well, I don't want to open that
discussion now, but clearly, at some point, we'll have the ability to
declare explicit patterns. This opens doors to writing your own
patterns that let you switch on arbitrary inputs:
switch (c) {
case isStringClass(): ...
case isIntegerClass(): ...
}
There's a long way to go to get there, and lots of ways to slice this,
but I think, if this problem is worth solving, there are other candidate
solutions that don't have the confusion downside.
On 4/10/2018 1:12 AM, Tagir Valeev wrote:
Hello!
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.
In any case I'm pretty sure that switch on class will be more
applicable, than the switch on floats. But you are doing theswitch 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.
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.
Several code samples found in JDK:
1. javafx.base/javafx/util/converter/LocalDateTimeStringConverter.java:197
(final classes)
if (type == LocalDate.class) {
return (T)LocalDate.from(chronology.date(temporal));
} else if (type == LocalTime.class) {
return (T)LocalTime.from(temporal);
} else {
return (T)LocalDateTime.from(chronology.localDateTime(temporal));
}
2. java.desktop/sun/print/Win32PrintService.java:928 (final classes)
if (category == ColorSupported.class) {
int caps = getPrinterCapabilities();
if ((caps & DEVCAP_COLOR) != 0) {
return (T)ColorSupported.SUPPORTED;
} else {
return (T)ColorSupported.NOT_SUPPORTED;
}
} else if (category == PrinterName.class) {
return (T)getPrinterName();
} else if (category == PrinterState.class) {
return (T)getPrinterState();
} else if (category == PrinterStateReasons.class) {
return (T)getPrinterStateReasons();
} else if (category == QueuedJobCount.class) {
return (T)getQueuedJobCount();
} else if (category == PrinterIsAcceptingJobs.class) {
return (T)getPrinterIsAcceptingJobs();
} else {
return null;
}
3. com.sun.media.sound.SoftSynthesizer#getPropertyInfo (line 926) -
final classes; several blocks like this
if (c == Byte.class)
item2.value = Byte.valueOf(s);
else if (c == Short.class)
item2.value = Short.valueOf(s);
else if (c == Integer.class)
item2.value = Integer.valueOf(s);
else if (c == Long.class)
item2.value = Long.valueOf(s);
else if (c == Float.class)
item2.value = Float.valueOf(s);
else if (c == Double.class)
item2.value = Double.valueOf(s);
4. java.awt.Component#getListeners (interfaces!), Window#getListeners,
List#getListeners, JComponent#getListeners, etc. are similar
public <T extends EventListener> T[] getListeners(Class<T>
listenerType) {
EventListener l = null;
if (listenerType == ComponentListener.class) {
l = componentListener;
} else if (listenerType == FocusListener.class) {
l = focusListener;
} else if (listenerType == HierarchyListener.class) {
l = hierarchyListener;
} else if (listenerType == HierarchyBoundsListener.class) {
l = hierarchyBoundsListener;
} else if (listenerType == KeyListener.class) {
l = keyListener;
} else if (listenerType == MouseListener.class) {
l = mouseListener;
} else if (listenerType == MouseMotionListener.class) {
l = mouseMotionListener;
} else if (listenerType == MouseWheelListener.class) {
l = mouseWheelListener;
} else if (listenerType == InputMethodListener.class) {
l = inputMethodListener;
} else if (listenerType == PropertyChangeListener.class) {
return (T[])getPropertyChangeListeners();
}
return AWTEventMulticaster.getListeners(l, listenerType);
}
5. java.beans.XMLEncoder#primitiveTypeFor (final classes)
if (wrapper == Boolean.class) return Boolean.TYPE;
if (wrapper == Byte.class) return Byte.TYPE;
if (wrapper == Character.class) return Character.TYPE;
if (wrapper == Short.class) return Short.TYPE;
if (wrapper == Integer.class) return Integer.TYPE;
if (wrapper == Long.class) return Long.TYPE;
if (wrapper == Float.class) return Float.TYPE;
if (wrapper == Double.class) return Double.TYPE;
if (wrapper == Void.class) return Void.TYPE;
return null;
6. javax.swing.plaf.synth.SynthTableUI.SynthTableCellRenderer#configureValue
(mix of abstract, non-final and final classes)
private void configureValue(Object value, Class<?> columnClass) {
if (columnClass == Object.class || columnClass == null) {
// case Object.class, null!
setHorizontalAlignment(JLabel.LEADING);
} else if (columnClass == Float.class || columnClass ==
Double.class) {
if (numberFormat == null) {
numberFormat = NumberFormat.getInstance();
}
setHorizontalAlignment(JLabel.TRAILING);
setText((value == null) ? "" :
((NumberFormat)numberFormat).format(value));
}
else if (columnClass == Number.class) {
setHorizontalAlignment(JLabel.TRAILING);
// Super will have set value.
}
else if (columnClass == Icon.class || columnClass ==
ImageIcon.class) {
setHorizontalAlignment(JLabel.CENTER);
setIcon((value instanceof Icon) ? (Icon)value : null);
setText("");
}
else if (columnClass == Date.class) {
if (dateFormat == null) {
dateFormat = DateFormat.getDateInstance();
}
setHorizontalAlignment(JLabel.LEADING);
setText((value == null) ? "" :
((Format)dateFormat).format(value));
}
else {
configureValue(value, columnClass.getSuperclass()); //
note this: recursively going to superclass automatically
}
}
With best regards,
Tagir Valeev.
On Mon, Apr 9, 2018 at 8:38 PM, Brian Goetz <brian.go...@oracle.com
<mailto:brian.go...@oracle.com>> wrote:
I'm skeptical of this feature, because (a) its not as widely
applicable as it looks, (b) its error-prone.
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.
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:
switch (date) {
case Date d: ...
case Timestamp t: ...
default: ...
}
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.
So, I would worry this feature is unlikely to carry its weight,
and further, may lead to misuse.
On 4/9/2018 1:07 AM, Tagir Valeev wrote:
Hello!
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:
1. com.google.gson.DefaultDateTypeAdapter#read (gson-2.8.2):
Date date = deserializeToDate(in.nextString());
if (dateType == Date.class) {
return date;
} else if (dateType == Timestamp.class) {
return new Timestamp(date.getTime());
} else if (dateType == java.sql.Date.class) {
return new java.sql.Date(date.getTime());
} else {
// This must never happen: dateType is guarded in the
primary constructor
throw new AssertionError();
}
Could be rewritten as:
Date date = deserializeToDate(in.nextString());
return switch(dateType) {
case Date.class -> date;
case Timestamp.class -> new Timestamp(date.getTime());
case java.sql.Date.class -> new
java.sql.Date(date.getTime());
default ->
// This must never happen: dateType is guarded in the
primary constructor
throw new AssertionError();
};
2.
com.fasterxml.jackson.databind.deser.std.FromStringDeserializer#findDeserializer
(jackson-databind-2.9.4):
public static Std findDeserializer(Class<?> rawType)
{
int kind = 0;
if (rawType == File.class) {
kind = Std.STD_FILE;
} else if (rawType == URL.class) {
kind = Std.STD_URL;
} else if (rawType == URI.class) {
kind = Std.STD_URI;
} else if (rawType == Class.class) {
kind = Std.STD_CLASS;
} else if (rawType == JavaType.class) {
kind = Std.STD_JAVA_TYPE;
} else if // more branches like this
} else {
return null;
}
return new Std(rawType, kind);
}
Could be rewritten as:
public static Std findDeserializer(Class<?> rawType)
{
int kind = switch(rawType) {
case File.class -> Std.STD_FILE;
case URL.class -> Std.STD_URL;
case URI.class -> Std.STD_URI;
case Class.cass -> Std.STD_CLASS;
case JavaType.class -> Std.STD_JAVA_TYPE;
...
default -> 0;
};
return kind == 0 ? null : new Std(rawType, kind);
}
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.
What do you think?
With best regards,
Tagir Valeev.
