You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by Remko Popma <re...@gmail.com> on 2014/04/17 00:40:55 UTC
Re: svn commit: r1587455 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java
Out of interest, what is the advantage of calling
clazz.getConstructor().newInstance();
vs
clazz.newInstance(); ?
Until now all the code I've seen simply uses newInstance unless the
constructor needs parameters.
And it is shorter. :-)
So just wondering if I missed something...
On Tue, Apr 15, 2014 at 4:03 PM, <ma...@apache.org> wrote:
> Author: mattsicker
> Date: Tue Apr 15 07:03:55 2014
> New Revision: 1587455
>
> URL: http://svn.apache.org/r1587455
> Log:
> Add fallback to Class.newInstance.
>
> - In some scenarios, if a class doesn't have a constructor defined,
> Class.getConstructor() will fail, but Class.newInstance() won't fail. Not
> sure on the logic behind that one, Java.
>
> Modified:
>
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java
>
> Modified:
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java
> URL:
> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java?rev=1587455&r1=1587454&r2=1587455&view=diff
>
> ==============================================================================
> ---
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java
> (original)
> +++
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java
> Tue Apr 15 07:03:55 2014
> @@ -21,6 +21,7 @@ import java.lang.ClassCastException;
> import java.lang.reflect.InvocationTargetException;
> import java.net.URL;
>
> +import org.apache.logging.log4j.Level;
> import org.apache.logging.log4j.Logger;
> import org.apache.logging.log4j.status.StatusLogger;
> import org.apache.logging.log4j.util.PropertiesUtil;
> @@ -248,7 +249,7 @@ public final class Loader {
> // using the TCCL should work the same as the default
> ClassLoader (i.e., init or not)
> return Class.forName(className, true, getTCL());
> } catch (final Throwable e) {
> - LOGGER.catching(e);
> + LOGGER.catching(Level.DEBUG, e);
> LOGGER.trace("TCCL didn't work. Trying Class.forName({}).",
> className);
> return loadClassWithDefaultClassLoader(className);
> }
> @@ -283,7 +284,7 @@ public final class Loader {
> try {
> return Class.forName(className, true,
> ClassLoader.getSystemClassLoader());
> } catch (final Throwable t) {
> - LOGGER.catching(t);
> + LOGGER.catching(Level.DEBUG, t);
> LOGGER.trace("Couldn't use SystemClassLoader. Trying
> Class.forName({}).", className);
> return Class.forName(className);
> }
> @@ -306,7 +307,13 @@ public final class Loader {
> InstantiationException,
> NoSuchMethodException,
> InvocationTargetException {
> - return loadClass(className).getConstructor().newInstance();
> + final Class<?> clazz = loadClass(className);
> + try {
> + return clazz.getConstructor().newInstance();
> + } catch (final NoSuchMethodException e) {
> + //noinspection ClassNewInstance
> + return clazz.newInstance();
> + }
> }
>
> /**
> @@ -342,7 +349,7 @@ public final class Loader {
> try {
> final Class<?> clazz = loadClass(className);
> return clazz != null;
> - } catch (final Exception ignored) {
> + } catch (final Throwable ignored) {
> return false;
> }
> }
>
>
>
Re: svn commit: r1587455 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java
Posted by Matt Sicker <bo...@gmail.com>.
I learned it from code quality tools first before looking it up. Basically,
beware when using reflection! :)
On 16 April 2014 17:22, Remko Popma <re...@gmail.com> wrote:
> I see, interesting!
>
> So Class.newInstance can throw, say, an SQLException even though the
> signature does not declare that exception.
>
> So if you have fine-grained exception handling (separate catch blocks for
> InstantiationException, IllegalAccessException, SecurityException etc)
> then you might might not catch that SQLException.
>
> I guess I've never run into this because I always use a single (catch
> Exception ex) block, and if reflection failed (for any reason) then I
> either fall back to plan B or just fail.
>
> So I guess it doesn't make much difference in our case.
>
> But I did learn something new today, thanks! :-)
>
> Remko
>
> Sent from my iPhone
>
> On 2014/04/17, at 8:00, Matt Sicker <bo...@gmail.com> wrote:
>
> From the docs for Class.newInstance:
>
> Note that this method propagates any exception thrown by the nullary
> constructor, including a checked exception. Use of this method effectively
> bypasses the compile-time exception checking that would otherwise be
> performed by the compiler. The Constructor.newInstance<http://docs.oracle.com/javase/7/docs/api/java/lang/reflect/Constructor.html#newInstance(java.lang.Object...)> method
> avoids this problem by wrapping any exception thrown by the constructor in
> a (checked) InvocationTargetException<http://docs.oracle.com/javase/7/docs/api/java/lang/reflect/InvocationTargetException.html>
> .
>
>
> On 16 April 2014 17:40, Remko Popma <re...@gmail.com> wrote:
>
>> Out of interest, what is the advantage of calling
>> clazz.getConstructor().newInstance();
>> vs
>> clazz.newInstance(); ?
>>
>> Until now all the code I've seen simply uses newInstance unless the
>> constructor needs parameters.
>> And it is shorter. :-)
>> So just wondering if I missed something...
>>
>>
>>
>> On Tue, Apr 15, 2014 at 4:03 PM, <ma...@apache.org> wrote:
>>
>>> Author: mattsicker
>>> Date: Tue Apr 15 07:03:55 2014
>>> New Revision: 1587455
>>>
>>> URL: http://svn.apache.org/r1587455
>>> Log:
>>> Add fallback to Class.newInstance.
>>>
>>> - In some scenarios, if a class doesn't have a constructor defined,
>>> Class.getConstructor() will fail, but Class.newInstance() won't fail. Not
>>> sure on the logic behind that one, Java.
>>>
>>> Modified:
>>>
>>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java
>>>
>>> Modified:
>>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java
>>> URL:
>>> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java?rev=1587455&r1=1587454&r2=1587455&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java
>>> (original)
>>> +++
>>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java
>>> Tue Apr 15 07:03:55 2014
>>> @@ -21,6 +21,7 @@ import java.lang.ClassCastException;
>>> import java.lang.reflect.InvocationTargetException;
>>> import java.net.URL;
>>>
>>> +import org.apache.logging.log4j.Level;
>>> import org.apache.logging.log4j.Logger;
>>> import org.apache.logging.log4j.status.StatusLogger;
>>> import org.apache.logging.log4j.util.PropertiesUtil;
>>> @@ -248,7 +249,7 @@ public final class Loader {
>>> // using the TCCL should work the same as the default
>>> ClassLoader (i.e., init or not)
>>> return Class.forName(className, true, getTCL());
>>> } catch (final Throwable e) {
>>> - LOGGER.catching(e);
>>> + LOGGER.catching(Level.DEBUG, e);
>>> LOGGER.trace("TCCL didn't work. Trying Class.forName({}).",
>>> className);
>>> return loadClassWithDefaultClassLoader(className);
>>> }
>>> @@ -283,7 +284,7 @@ public final class Loader {
>>> try {
>>> return Class.forName(className, true,
>>> ClassLoader.getSystemClassLoader());
>>> } catch (final Throwable t) {
>>> - LOGGER.catching(t);
>>> + LOGGER.catching(Level.DEBUG, t);
>>> LOGGER.trace("Couldn't use SystemClassLoader. Trying
>>> Class.forName({}).", className);
>>> return Class.forName(className);
>>> }
>>> @@ -306,7 +307,13 @@ public final class Loader {
>>> InstantiationException,
>>> NoSuchMethodException,
>>> InvocationTargetException {
>>> - return loadClass(className).getConstructor().newInstance();
>>> + final Class<?> clazz = loadClass(className);
>>> + try {
>>> + return clazz.getConstructor().newInstance();
>>> + } catch (final NoSuchMethodException e) {
>>> + //noinspection ClassNewInstance
>>> + return clazz.newInstance();
>>> + }
>>> }
>>>
>>> /**
>>> @@ -342,7 +349,7 @@ public final class Loader {
>>> try {
>>> final Class<?> clazz = loadClass(className);
>>> return clazz != null;
>>> - } catch (final Exception ignored) {
>>> + } catch (final Throwable ignored) {
>>> return false;
>>> }
>>> }
>>>
>>>
>>>
>>
>
>
> --
> Matt Sicker <bo...@gmail.com>
>
>
--
Matt Sicker <bo...@gmail.com>
Re: svn commit: r1587455 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java
Posted by Remko Popma <re...@gmail.com>.
I see, interesting!
So Class.newInstance can throw, say, an SQLException even though the signature does not declare that exception.
So if you have fine-grained exception handling (separate catch blocks for InstantiationException, IllegalAccessException, SecurityException etc) then you might might not catch that SQLException.
I guess I've never run into this because I always use a single (catch Exception ex) block, and if reflection failed (for any reason) then I either fall back to plan B or just fail.
So I guess it doesn't make much difference in our case.
But I did learn something new today, thanks! :-)
Remko
Sent from my iPhone
> On 2014/04/17, at 8:00, Matt Sicker <bo...@gmail.com> wrote:
>
> From the docs for Class.newInstance:
>
> Note that this method propagates any exception thrown by the nullary constructor, including a checked exception. Use of this method effectively bypasses the compile-time exception checking that would otherwise be performed by the compiler. The Constructor.newInstance method avoids this problem by wrapping any exception thrown by the constructor in a (checked) InvocationTargetException.
>
>
>> On 16 April 2014 17:40, Remko Popma <re...@gmail.com> wrote:
>> Out of interest, what is the advantage of calling
>> clazz.getConstructor().newInstance();
>> vs
>> clazz.newInstance(); ?
>>
>> Until now all the code I've seen simply uses newInstance unless the constructor needs parameters.
>> And it is shorter. :-)
>> So just wondering if I missed something...
>>
>>
>>
>>> On Tue, Apr 15, 2014 at 4:03 PM, <ma...@apache.org> wrote:
>>> Author: mattsicker
>>> Date: Tue Apr 15 07:03:55 2014
>>> New Revision: 1587455
>>>
>>> URL: http://svn.apache.org/r1587455
>>> Log:
>>> Add fallback to Class.newInstance.
>>>
>>> - In some scenarios, if a class doesn't have a constructor defined, Class.getConstructor() will fail, but Class.newInstance() won't fail. Not sure on the logic behind that one, Java.
>>>
>>> Modified:
>>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java
>>>
>>> Modified: logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java
>>> URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java?rev=1587455&r1=1587454&r2=1587455&view=diff
>>> ==============================================================================
>>> --- logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java (original)
>>> +++ logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java Tue Apr 15 07:03:55 2014
>>> @@ -21,6 +21,7 @@ import java.lang.ClassCastException;
>>> import java.lang.reflect.InvocationTargetException;
>>> import java.net.URL;
>>>
>>> +import org.apache.logging.log4j.Level;
>>> import org.apache.logging.log4j.Logger;
>>> import org.apache.logging.log4j.status.StatusLogger;
>>> import org.apache.logging.log4j.util.PropertiesUtil;
>>> @@ -248,7 +249,7 @@ public final class Loader {
>>> // using the TCCL should work the same as the default ClassLoader (i.e., init or not)
>>> return Class.forName(className, true, getTCL());
>>> } catch (final Throwable e) {
>>> - LOGGER.catching(e);
>>> + LOGGER.catching(Level.DEBUG, e);
>>> LOGGER.trace("TCCL didn't work. Trying Class.forName({}).", className);
>>> return loadClassWithDefaultClassLoader(className);
>>> }
>>> @@ -283,7 +284,7 @@ public final class Loader {
>>> try {
>>> return Class.forName(className, true, ClassLoader.getSystemClassLoader());
>>> } catch (final Throwable t) {
>>> - LOGGER.catching(t);
>>> + LOGGER.catching(Level.DEBUG, t);
>>> LOGGER.trace("Couldn't use SystemClassLoader. Trying Class.forName({}).", className);
>>> return Class.forName(className);
>>> }
>>> @@ -306,7 +307,13 @@ public final class Loader {
>>> InstantiationException,
>>> NoSuchMethodException,
>>> InvocationTargetException {
>>> - return loadClass(className).getConstructor().newInstance();
>>> + final Class<?> clazz = loadClass(className);
>>> + try {
>>> + return clazz.getConstructor().newInstance();
>>> + } catch (final NoSuchMethodException e) {
>>> + //noinspection ClassNewInstance
>>> + return clazz.newInstance();
>>> + }
>>> }
>>>
>>> /**
>>> @@ -342,7 +349,7 @@ public final class Loader {
>>> try {
>>> final Class<?> clazz = loadClass(className);
>>> return clazz != null;
>>> - } catch (final Exception ignored) {
>>> + } catch (final Throwable ignored) {
>>> return false;
>>> }
>>> }
>
>
>
> --
> Matt Sicker <bo...@gmail.com>
Re: svn commit: r1587455 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java
Posted by Matt Sicker <bo...@gmail.com>.
>From the docs for Class.newInstance:
Note that this method propagates any exception thrown by the nullary
constructor, including a checked exception. Use of this method effectively
bypasses the compile-time exception checking that would otherwise be
performed by the compiler. The
Constructor.newInstance<http://docs.oracle.com/javase/7/docs/api/java/lang/reflect/Constructor.html#newInstance(java.lang.Object...)>
method
avoids this problem by wrapping any exception thrown by the constructor in
a (checked) InvocationTargetException<http://docs.oracle.com/javase/7/docs/api/java/lang/reflect/InvocationTargetException.html>
.
On 16 April 2014 17:40, Remko Popma <re...@gmail.com> wrote:
> Out of interest, what is the advantage of calling
> clazz.getConstructor().newInstance();
> vs
> clazz.newInstance(); ?
>
> Until now all the code I've seen simply uses newInstance unless the
> constructor needs parameters.
> And it is shorter. :-)
> So just wondering if I missed something...
>
>
>
> On Tue, Apr 15, 2014 at 4:03 PM, <ma...@apache.org> wrote:
>
>> Author: mattsicker
>> Date: Tue Apr 15 07:03:55 2014
>> New Revision: 1587455
>>
>> URL: http://svn.apache.org/r1587455
>> Log:
>> Add fallback to Class.newInstance.
>>
>> - In some scenarios, if a class doesn't have a constructor defined,
>> Class.getConstructor() will fail, but Class.newInstance() won't fail. Not
>> sure on the logic behind that one, Java.
>>
>> Modified:
>>
>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java
>>
>> Modified:
>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java
>> URL:
>> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java?rev=1587455&r1=1587454&r2=1587455&view=diff
>>
>> ==============================================================================
>> ---
>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java
>> (original)
>> +++
>> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/helpers/Loader.java
>> Tue Apr 15 07:03:55 2014
>> @@ -21,6 +21,7 @@ import java.lang.ClassCastException;
>> import java.lang.reflect.InvocationTargetException;
>> import java.net.URL;
>>
>> +import org.apache.logging.log4j.Level;
>> import org.apache.logging.log4j.Logger;
>> import org.apache.logging.log4j.status.StatusLogger;
>> import org.apache.logging.log4j.util.PropertiesUtil;
>> @@ -248,7 +249,7 @@ public final class Loader {
>> // using the TCCL should work the same as the default
>> ClassLoader (i.e., init or not)
>> return Class.forName(className, true, getTCL());
>> } catch (final Throwable e) {
>> - LOGGER.catching(e);
>> + LOGGER.catching(Level.DEBUG, e);
>> LOGGER.trace("TCCL didn't work. Trying Class.forName({}).",
>> className);
>> return loadClassWithDefaultClassLoader(className);
>> }
>> @@ -283,7 +284,7 @@ public final class Loader {
>> try {
>> return Class.forName(className, true,
>> ClassLoader.getSystemClassLoader());
>> } catch (final Throwable t) {
>> - LOGGER.catching(t);
>> + LOGGER.catching(Level.DEBUG, t);
>> LOGGER.trace("Couldn't use SystemClassLoader. Trying
>> Class.forName({}).", className);
>> return Class.forName(className);
>> }
>> @@ -306,7 +307,13 @@ public final class Loader {
>> InstantiationException,
>> NoSuchMethodException,
>> InvocationTargetException {
>> - return loadClass(className).getConstructor().newInstance();
>> + final Class<?> clazz = loadClass(className);
>> + try {
>> + return clazz.getConstructor().newInstance();
>> + } catch (final NoSuchMethodException e) {
>> + //noinspection ClassNewInstance
>> + return clazz.newInstance();
>> + }
>> }
>>
>> /**
>> @@ -342,7 +349,7 @@ public final class Loader {
>> try {
>> final Class<?> clazz = loadClass(className);
>> return clazz != null;
>> - } catch (final Exception ignored) {
>> + } catch (final Throwable ignored) {
>> return false;
>> }
>> }
>>
>>
>>
>
--
Matt Sicker <bo...@gmail.com>