Crud, I guess that's a no go. Gary On Feb 28, 2016 6:13 PM, "Matt Sicker" <[email protected]> wrote:
> That depends if changing from IllegalArgumentException to > NullPointerException in the Marker interface is acceptable. > > On 28 February 2016 at 20:05, Gary Gregory <[email protected]> wrote: > >> How about using java.lang.Objects? >> >> Gary >> ---------- Forwarded message ---------- >> From: <[email protected]> >> Date: Feb 28, 2016 5:49 PM >> Subject: [2/2] logging-log4j2 git commit: Extract a requireNonNull method. >> To: <[email protected]> >> Cc: >> >> Extract a requireNonNull method. >> >> In the past, these method calls could have simply used >> Objects.requireNonNull, but a later revision switched them to use >> IllegalArgumentException. >> >> >> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo >> Commit: >> http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/9d63b0d1 >> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/9d63b0d1 >> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/9d63b0d1 >> >> Branch: refs/heads/master >> Commit: 9d63b0d11fe3d24b0b3d670ab7f3b369c0f6e1dd >> Parents: 96dff0c >> Author: Matt Sicker <[email protected]> >> Authored: Sun Feb 28 19:49:43 2016 -0600 >> Committer: Matt Sicker <[email protected]> >> Committed: Sun Feb 28 19:49:43 2016 -0600 >> >> ---------------------------------------------------------------------- >> .../org/apache/logging/log4j/MarkerManager.java | 40 +++++++++----------- >> 1 file changed, 18 insertions(+), 22 deletions(-) >> ---------------------------------------------------------------------- >> >> >> >> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/9d63b0d1/log4j-api/src/main/java/org/apache/logging/log4j/MarkerManager.java >> ---------------------------------------------------------------------- >> diff --git >> a/log4j-api/src/main/java/org/apache/logging/log4j/MarkerManager.java >> b/log4j-api/src/main/java/org/apache/logging/log4j/MarkerManager.java >> index d2b3d19..d57c483 100644 >> --- a/log4j-api/src/main/java/org/apache/logging/log4j/MarkerManager.java >> +++ b/log4j-api/src/main/java/org/apache/logging/log4j/MarkerManager.java >> @@ -42,7 +42,7 @@ public final class MarkerManager { >> >> /** >> * Tests existence of the given marker. >> - * >> + * >> * @param key the marker name >> * @return true if the marker exists. >> * @since 2.4 >> @@ -53,7 +53,7 @@ public final class MarkerManager { >> >> /** >> * Retrieves a Marker or create a Marker that has no parent. >> - * >> + * >> * @param name The name of the Marker. >> * @return The Marker with the specified name. >> * @throws IllegalArgumentException if the argument is {@code null} >> @@ -65,7 +65,7 @@ public final class MarkerManager { >> >> /** >> * Retrieves or creates a Marker with the specified parent. The >> parent must have been previously created. >> - * >> + * >> * @param name The name of the Marker. >> * @param parent The name of the parent Marker. >> * @return The Marker with the specified name. >> @@ -85,7 +85,7 @@ public final class MarkerManager { >> >> /** >> * Retrieves or creates a Marker with the specified parent. >> - * >> + * >> * @param name The name of the Marker. >> * @param parent The parent Marker. >> * @return The Marker with the specified name. >> @@ -128,16 +128,14 @@ public final class MarkerManager { >> >> /** >> * Constructs a new Marker. >> - * >> + * >> * @param name the name of the Marker. >> * @throws IllegalArgumentException if the argument is {@code >> null} >> */ >> public Log4jMarker(final String name) { >> - if (name == null) { >> - // we can't store null references in a ConcurrentHashMap >> as it is, not to mention that a null Marker >> - // name seems rather pointless. To get an "anonymous" >> Marker, just use an empty string. >> - throw new IllegalArgumentException("Marker name cannot >> be null."); >> - } >> + // we can't store null references in a ConcurrentHashMap as >> it is, not to mention that a null Marker >> + // name seems rather pointless. To get an "anonymous" >> Marker, just use an empty string. >> + requireNonNull(name, "Marker name cannot be null."); >> this.name = name; >> this.parents = null; >> } >> @@ -146,9 +144,7 @@ public final class MarkerManager { >> >> @Override >> public synchronized Marker addParents(final Marker... >> parentMarkers) { >> - if (parentMarkers == null) { >> - throw new IllegalArgumentException("A parent marker must >> be specified"); >> - } >> + requireNonNull(parentMarkers, "A parent marker must be >> specified"); >> // It is not strictly necessary to copy the variable here >> but it should perform better than >> // Accessing a volatile variable multiple times. >> final Marker[] localParents = this.parents; >> @@ -184,9 +180,7 @@ public final class MarkerManager { >> >> @Override >> public synchronized boolean remove(final Marker parent) { >> - if (parent == null) { >> - throw new IllegalArgumentException("A parent marker must >> be specified"); >> - } >> + requireNonNull(parent, "A parent marker must be specified"); >> final Marker[] localParents = this.parents; >> if (localParents == null) { >> return false; >> @@ -249,9 +243,7 @@ public final class MarkerManager { >> @Override >> @PerformanceSensitive({"allocation", "unrolled"}) >> public boolean isInstanceOf(final Marker marker) { >> - if (marker == null) { >> - throw new IllegalArgumentException("A marker parameter >> is required"); >> - } >> + requireNonNull(marker, "A marker parameter is required"); >> if (this == marker) { >> return true; >> } >> @@ -279,9 +271,7 @@ public final class MarkerManager { >> @Override >> @PerformanceSensitive({"allocation", "unrolled"}) >> public boolean isInstanceOf(final String markerName) { >> - if (markerName == null) { >> - throw new IllegalArgumentException("A marker name is >> required"); >> - } >> + requireNonNull(markerName, "A marker name is required"); >> if (markerName.equals(this.getName())) { >> return true; >> } >> @@ -402,4 +392,10 @@ public final class MarkerManager { >> } >> } >> >> + // this method wouldn't be necessary if Marker methods threw an NPE >> instead of an IAE for null values ;) >> + private static void requireNonNull(final Object obj, final String >> message) { >> + if (obj == null) { >> + throw new IllegalArgumentException(message); >> + } >> + } >> } >> >> > > > -- > Matt Sicker <[email protected]> >
