On Mon, 21 Feb 2022 12:16:53 GMT, Andrey Turbanov <aturba...@openjdk.org> wrote:
>> I've stared at the javadoc for Class.isAssignableFrom and Class.isInstance >> and if a non-null instance is used to get a non-null class they are PROBABLY >> the same but it is far from clear. The implementations of both are at least >> native and may be instrinsicified. The doc for Class.isAssignableFrom cites >> JLS 5.1.4 which in what I found is about primitives so I suspect it is >> woefully out of date >> https://docs.oracle.com/javase/specs/jls/se17/html/jls-5.html#jls-5.1.4 >> >> What client tests have you run that touch the code you are changing ? >> >> In short I see insufficient value in the changes here and would prefer you >> drop the client part so I don't have to worry about it. > >>In short I see insufficient value in the changes here and would prefer you >>drop the client part so I don't have to worry about it. > > I think, usage of `isInstance` is much clear for most java developers. > Everyone knows about java _instanceof_ operator. And `isInstance` method > javadoc is very straight forward: `This method is the dynamic equivalent of > the Java language instanceof operator.` > > Method `isAssignableFrom` is opposite: it brings unnecessary complexity in > the code. And it's easy to confuse orders of parameters. Even JBS confirms > that: > 1. [Wrong isAssignableFrom test when adding Principal to Subject > ](https://bugs.openjdk.java.net/browse/JDK-8034820) > 2. [isAssignableFrom checks in KeyFactorySpi.engineGetKeySpec appear to be > backwards](https://bugs.openjdk.java.net/browse/JDK-8254717) > 3. [isAssignableFrom checks in AlgorithmParametersSpi.engineGetParameterSpec > appear to be backwards](https://bugs.openjdk.java.net/browse/JDK-8279800) > > So, it gives all 3 points to prefer isInstance method: it's shorter, it's > clearer for most java developers, it's faster. > @turbanoff > > Method `isAssignableFrom` is opposite: it brings unnecessary complexity in > > the code. And it's easy to confuse orders of parameters. Even JBS confirms > > that: > > Maybe we should add `Class::isSubclassOf(Class<?> that)` that performs > `that.isAssignableFrom(this)`: I have opened [JI‑9073064](https://bugs.openjdk.java.net/browse/JI-9073064 "[JI‑9073064] Add the method `Class::isSublassOf(Class<?>)` to `java.lang.Class` that does the inverse of `Class::isAssignableFrom`") for this. ------------- PR: https://git.openjdk.java.net/jdk/pull/7061
