Why did you make the change? Was something broken?

Ralph

> On Dec 2, 2014, at 3:08 PM, Gary Gregory <[email protected]> wrote:
> 
> 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] 
> <mailto:[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] <mailto:[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 
> > <http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo>
> > Commit: 
> > http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/df71c934 
> > <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 
> > <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 
> > <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] 
> > <mailto:[email protected]>>
> > Authored: Tue Dec 2 13:10:10 2014 -0500
> > Committer: Gary Gregory <[email protected] 
> > <mailto:[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
> >  
> > <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] 
> <mailto:[email protected]>
> For additional commands, e-mail: [email protected] 
> <mailto:[email protected]>
> 
> 
> 
> 
> -- 
> E-Mail: [email protected] <mailto:[email protected]> | 
> [email protected]  <mailto:[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 <http://garygregory.wordpress.com/> 
> Home: http://garygregory.com/ <http://garygregory.com/>
> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory>

Reply via email to