Yes: This is a change that refactors guard clauses, no behavioral change.
Tests pass.
No: I do not know if the specific case you mention is covered by the unit
tests or the code for that matter. The tests do pass though.
Thoughts?

Gary

On Tue, Dec 2, 2014 at 5:01 PM, Remko Popma <[email protected]> wrote:

> Are you sure this is correct?
> What about deserialized ThrowableProxies where the cause is null but the
> causeProxy is non-null?
>
> Sent from my iPhone
>
> > On 2014/12/03, at 3:10, [email protected] wrote:
> >
> > Repository: logging-log4j2
> > Updated Branches:
> >  refs/heads/master ab58a6d69 -> df71c934d
> >
> >
> > Let the formatCause method have a null guard clause instead of each call
> > site guarding the call.
> >
> > Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
> > Commit:
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/df71c934
> > Tree:
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/df71c934
> > Diff:
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/df71c934
> >
> > Branch: refs/heads/master
> > Commit: df71c934d59570bbee10d8eb413d92eaa443bec4
> > Parents: ab58a6d
> > Author: Gary Gregory <[email protected]>
> > Authored: Tue Dec 2 13:10:10 2014 -0500
> > Committer: Gary Gregory <[email protected]>
> > Committed: Tue Dec 2 13:10:10 2014 -0500
> >
> > ----------------------------------------------------------------------
> > .../apache/logging/log4j/core/impl/ThrowableProxy.java   | 11 +++++------
> > 1 file changed, 5 insertions(+), 6 deletions(-)
> > ----------------------------------------------------------------------
> >
> >
> >
> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/df71c934/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java
> > ----------------------------------------------------------------------
> > diff --git
> a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java
> b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java
> > index 3aadf6c..23065ab 100644
> > ---
> a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java
> > +++
> b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThrowableProxy.java
> > @@ -183,12 +183,13 @@ public class ThrowableProxy implements
> Serializable {
> >
> >     @SuppressWarnings("ThrowableResultOfMethodCallIgnored")
> >     private void formatCause(final StringBuilder sb, final
> ThrowableProxy cause, final List<String> ignorePackages) {
> > +        if (cause == null) {
> > +            return;
> > +        }
> >         sb.append("Caused by: ").append(cause).append(EOL);
> >         this.formatElements(sb, cause.commonElementCount,
> cause.getThrowable().getStackTrace(),
> >                 cause.extendedStackTrace, ignorePackages);
> > -        if (cause.getCauseProxy() != null) {
> > -            this.formatCause(sb, cause.causeProxy, ignorePackages);
> > -        }
> > +        this.formatCause(sb, cause.causeProxy, ignorePackages);
> >     }
> >
> >     private void formatElements(final StringBuilder sb, final int
> commonCount, final StackTraceElement[] causedTrace,
> > @@ -342,9 +343,7 @@ public class ThrowableProxy implements Serializable {
> >         }
> >         sb.append('\n');
> >         this.formatElements(sb, 0, this.throwable.getStackTrace(),
> this.extendedStackTrace, ignorePackages);
> > -        if (this.causeProxy != null) {
> > -            this.formatCause(sb, this.causeProxy, ignorePackages);
> > -        }
> > +        this.formatCause(sb, this.causeProxy, ignorePackages);
> >         return sb.toString();
> >     }
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>
>


-- 
E-Mail: [email protected] | [email protected]
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Reply via email to