On Tue, Dec 2, 2014 at 6:37 PM, Ralph Goers <[email protected]> wrote:
> Why did you make the change? > Because I do not like code duplication and we had null checks before calling a method. It looks less cluttered now IMO. > Was something broken? > Not for the normal definition of "broken". But we don't have to only make changes to "fix" something... Gary > > 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]> 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 > > > -- 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
