[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8280035: Use Class.isInstance instead of Class.isAssignableFrom where applicable
From:       ExE Boss <duke () openjdk ! java ! net>
Date:       2022-04-15 13:47:40
Message-ID: QSBRL0wFUJe_ClWr0TOwOMqi-JpvGCbsGgmFkt-MdzE=.34d57b2f-9e6f-430e-9e55-c798aa8cedf8 () github ! com
[Download RAW message or body]

On Mon, 21 Feb 2022 12:16:53 GMT, Andrey Turbanov <aturbanov@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


[prev in list] [next in list] [prev in thread] [next in thread]