You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pivot.apache.org by rw...@apache.org on 2015/11/03 22:44:48 UTC
svn commit: r1712423 - in /pivot/trunk:
core/src/org/apache/pivot/util/Utils.java
wtk/src/org/apache/pivot/wtk/Bounds.java
wtk/src/org/apache/pivot/wtk/Component.java
wtk/src/org/apache/pivot/wtk/Window.java
Author: rwhitcomb
Date: Tue Nov 3 21:44:48 2015
New Revision: 1712423
URL: http://svn.apache.org/viewvc?rev=1712423&view=rev
Log:
Code cleanup: Make a new method in the org.apache.pivot.utils.Utils class which does the
common operation of checking an argument for null and throwing "IllegalArgumentException"
if it is. This will simplify and standardize a lot of code, since this pattern appears
so often, whenever objects are passed as arguments. And it will simplify getting an
understandable message thrown out in every case. Before, there were many places that
just threw the plain exception without a detail message, but this change will simplify
the checking so there can always be a meaningful message.
Change just a few files in the "wtk" area that do a lot of this checking already.
Of course, there are many more places that could use this treatment...
Modified:
pivot/trunk/core/src/org/apache/pivot/util/Utils.java
pivot/trunk/wtk/src/org/apache/pivot/wtk/Bounds.java
pivot/trunk/wtk/src/org/apache/pivot/wtk/Component.java
pivot/trunk/wtk/src/org/apache/pivot/wtk/Window.java
Modified: pivot/trunk/core/src/org/apache/pivot/util/Utils.java
URL: http://svn.apache.org/viewvc/pivot/trunk/core/src/org/apache/pivot/util/Utils.java?rev=1712423&r1=1712422&r2=1712423&view=diff
==============================================================================
--- pivot/trunk/core/src/org/apache/pivot/util/Utils.java (original)
+++ pivot/trunk/core/src/org/apache/pivot/util/Utils.java Tue Nov 3 21:44:48 2015
@@ -38,4 +38,26 @@ public class Utils {
return s1.equals(s2);
}
+ /**
+ * Check if the input argument is {@code null} and throw an
+ * {@link IllegalArgumentException} if so, with an optional
+ * message derived from the given string.
+ *
+ * @param value The argument value to check for {@code null}.
+ * @param description A description for the value used to
+ * construct a message like {@code "xxx must not be null."}. Can be
+ * {@code null} in which case a plain {@link IllegalArgumentException}
+ * is thrown without any detail message.
+ * @throws IllegalArgumentException if the value is {@code null}.
+ */
+ public static void checkNull(Object value, String description) {
+ if (value == null) {
+ if (description == null) {
+ throw new IllegalArgumentException();
+ } else {
+ throw new IllegalArgumentException(description + " must not be null.");
+ }
+ }
+ }
+
}
Modified: pivot/trunk/wtk/src/org/apache/pivot/wtk/Bounds.java
URL: http://svn.apache.org/viewvc/pivot/trunk/wtk/src/org/apache/pivot/wtk/Bounds.java?rev=1712423&r1=1712422&r2=1712423&view=diff
==============================================================================
--- pivot/trunk/wtk/src/org/apache/pivot/wtk/Bounds.java (original)
+++ pivot/trunk/wtk/src/org/apache/pivot/wtk/Bounds.java Tue Nov 3 21:44:48 2015
@@ -21,6 +21,7 @@ import java.io.Serializable;
import org.apache.pivot.collections.Dictionary;
import org.apache.pivot.json.JSONSerializer;
import org.apache.pivot.serialization.SerializationException;
+import org.apache.pivot.util.Utils;
/**
* Class representing the bounds of an object.
@@ -46,13 +47,8 @@ public final class Bounds implements Ser
}
public Bounds(Point origin, Dimensions size) {
- if (origin == null) {
- throw new IllegalArgumentException("origin is null.");
- }
-
- if (size == null) {
- throw new IllegalArgumentException("size is null.");
- }
+ Utils.checkNull(origin, "origin");
+ Utils.checkNull(size, "size");
x = origin.x;
y = origin.y;
@@ -61,9 +57,7 @@ public final class Bounds implements Ser
}
public Bounds(Bounds bounds) {
- if (bounds == null) {
- throw new IllegalArgumentException("bounds is null.");
- }
+ Utils.checkNull(bounds, "bounds");
x = bounds.x;
y = bounds.y;
@@ -72,9 +66,7 @@ public final class Bounds implements Ser
}
public Bounds(Dictionary<String, ?> bounds) {
- if (bounds == null) {
- throw new IllegalArgumentException("bounds is null.");
- }
+ Utils.checkNull(bounds, "bounds");
if (bounds.containsKey(X_KEY)) {
x = ((Integer) bounds.get(X_KEY)).intValue();
@@ -102,9 +94,7 @@ public final class Bounds implements Ser
}
public Bounds(java.awt.Rectangle rectangle) {
- if (rectangle == null) {
- throw new IllegalArgumentException("rectangle is null.");
- }
+ Utils.checkNull(rectangle, "rectangle");
x = rectangle.x;
y = rectangle.y;
@@ -160,9 +150,7 @@ public final class Bounds implements Ser
}
public boolean contains(Point point) {
- if (point == null) {
- throw new IllegalArgumentException("point is null");
- }
+ Utils.checkNull(point, "point");
return contains(point.x, point.y);
}
@@ -173,9 +161,7 @@ public final class Bounds implements Ser
}
public boolean contains(Bounds bounds) {
- if (bounds == null) {
- throw new IllegalArgumentException("bounds is null");
- }
+ Utils.checkNull(bounds, "bounds");
return contains(bounds.x, bounds.y, bounds.width, bounds.height);
}
@@ -187,9 +173,7 @@ public final class Bounds implements Ser
}
public boolean intersects(Bounds bounds) {
- if (bounds == null) {
- throw new IllegalArgumentException("bounds is null");
- }
+ Utils.checkNull(bounds, "bounds");
return intersects(bounds.x, bounds.y, bounds.width, bounds.height);
}
@@ -236,14 +220,12 @@ public final class Bounds implements Ser
return getClass().getName() + " [" + x + "," + y + ";" + width + "x" + height + "]";
}
- public static Bounds decode(String value) {
- if (value == null) {
- throw new IllegalArgumentException();
- }
+ public static Bounds decode(String boundsValue) {
+ Utils.checkNull(boundsValue, "boundsValue");
Bounds bounds;
try {
- bounds = new Bounds(JSONSerializer.parseMap(value));
+ bounds = new Bounds(JSONSerializer.parseMap(boundsValue));
} catch (SerializationException exception) {
throw new IllegalArgumentException(exception);
}
Modified: pivot/trunk/wtk/src/org/apache/pivot/wtk/Component.java
URL: http://svn.apache.org/viewvc/pivot/trunk/wtk/src/org/apache/pivot/wtk/Component.java?rev=1712423&r1=1712422&r2=1712423&view=diff
==============================================================================
--- pivot/trunk/wtk/src/org/apache/pivot/wtk/Component.java (original)
+++ pivot/trunk/wtk/src/org/apache/pivot/wtk/Component.java Tue Nov 3 21:44:48 2015
@@ -35,6 +35,7 @@ import org.apache.pivot.json.JSONSeriali
import org.apache.pivot.serialization.SerializationException;
import org.apache.pivot.util.ImmutableIterator;
import org.apache.pivot.util.ListenerList;
+import org.apache.pivot.util.Utils;
import org.apache.pivot.wtk.effects.Decorator;
/**
@@ -177,9 +178,7 @@ public abstract class Component implemen
@Override
public void insert(Decorator decorator, int index) {
- if (decorator == null) {
- throw new IllegalArgumentException("decorator is null");
- }
+ Utils.checkNull(decorator, "decorator");
// Repaint the the component's previous decorated region
if (parent != null) {
@@ -198,9 +197,7 @@ public abstract class Component implemen
@Override
public Decorator update(int index, Decorator decorator) {
- if (decorator == null) {
- throw new IllegalArgumentException("decorator is null.");
- }
+ Utils.checkNull(decorator, "decorator");
// Repaint the the component's previous decorated region
if (parent != null) {
@@ -744,9 +741,7 @@ public abstract class Component implemen
*/
@SuppressWarnings("unchecked")
protected void setSkin(Skin skin) {
- if (skin == null) {
- throw new IllegalArgumentException("skin is null.");
- }
+ Utils.checkNull(skin, "skin");
if (this.skin != null) {
throw new IllegalStateException("Skin is already installed.");
@@ -868,9 +863,7 @@ public abstract class Component implemen
@SuppressWarnings("unchecked")
public Container getAncestor(String ancestorTypeName) throws ClassNotFoundException {
- if (ancestorTypeName == null) {
- throw new IllegalArgumentException();
- }
+ Utils.checkNull(ancestorTypeName, "ancestorTypeName");
return getAncestor((Class<? extends Container>) Class.forName(ancestorTypeName));
}
@@ -898,9 +891,7 @@ public abstract class Component implemen
}
public final void setSize(Dimensions size) {
- if (size == null) {
- throw new IllegalArgumentException("size is null.");
- }
+ Utils.checkNull(size, "size");
setSize(size.width, size.height);
}
@@ -1105,9 +1096,7 @@ public abstract class Component implemen
}
public final void setPreferredSize(Dimensions preferredSize) {
- if (preferredSize == null) {
- throw new IllegalArgumentException("preferredSize is null.");
- }
+ Utils.checkNull(preferredSize, "preferredSize");
setPreferredSize(preferredSize.width, preferredSize.height);
}
@@ -1232,9 +1221,7 @@ public abstract class Component implemen
* @param widthLimits The new width limits (min and max).
*/
public final void setWidthLimits(Limits widthLimits) {
- if (widthLimits == null) {
- throw new IllegalArgumentException("widthLimits is null.");
- }
+ Utils.checkNull(widthLimits, "widthLimits");
setWidthLimits(widthLimits.minimum, widthLimits.maximum);
}
@@ -1316,9 +1303,7 @@ public abstract class Component implemen
* @param heightLimits The new height limits (min and max).
*/
public final void setHeightLimits(Limits heightLimits) {
- if (heightLimits == null) {
- throw new IllegalArgumentException("heightLimits is null.");
- }
+ Utils.checkNull(heightLimits, "heightLimits");
setHeightLimits(heightLimits.minimum, heightLimits.maximum);
}
@@ -1415,9 +1400,7 @@ public abstract class Component implemen
* @see #setLocation(int, int)
*/
public final void setLocation(Point location) {
- if (location == null) {
- throw new IllegalArgumentException("location cannot be null.");
- }
+ Utils.checkNull(location, "location");
setLocation(location.x, location.y);
}
@@ -1568,9 +1551,7 @@ public abstract class Component implemen
* the component is not a descendant of the specified ancestor.
*/
public Point mapPointToAncestor(final Container ancestor, int xArgument, int yArgument) {
- if (ancestor == null) {
- throw new IllegalArgumentException("ancestor is null");
- }
+ Utils.checkNull(ancestor, "ancestor");
int xArgumentMutable = xArgument;
int yArgumentMutable = yArgument;
@@ -1603,9 +1584,7 @@ public abstract class Component implemen
* the component is not a descendant of the specified ancestor.
*/
public Point mapPointToAncestor(Container ancestor, Point location) {
- if (location == null) {
- throw new IllegalArgumentException();
- }
+ Utils.checkNull(location, "location");
return mapPointToAncestor(ancestor, location.x, location.y);
}
@@ -1621,9 +1600,7 @@ public abstract class Component implemen
* the component is not a descendant of the specified ancestor.
*/
public Point mapPointFromAncestor(final Container ancestor, int xArgument, int yArgument) {
- if (ancestor == null) {
- throw new IllegalArgumentException("ancestor is null");
- }
+ Utils.checkNull(ancestor, "ancestor");
int xArgumentMutable = xArgument;
int yArgumentMutable = yArgument;
@@ -1646,9 +1623,7 @@ public abstract class Component implemen
}
public Point mapPointFromAncestor(Container ancestor, Point location) {
- if (location == null) {
- throw new IllegalArgumentException();
- }
+ Utils.checkNull(location, "location");
return mapPointFromAncestor(ancestor, location.x, location.y);
}
@@ -1694,9 +1669,7 @@ public abstract class Component implemen
* part of the component hierarchy.
*/
public Bounds getVisibleArea(Bounds area) {
- if (area == null) {
- throw new IllegalArgumentException("area is null.");
- }
+ Utils.checkNull(area, "area");
return getVisibleArea(area.x, area.y, area.width, area.height);
}
@@ -1768,9 +1741,7 @@ public abstract class Component implemen
* @param area The area to be made visible.
*/
public void scrollAreaToVisible(Bounds area) {
- if (area == null) {
- throw new IllegalArgumentException("area is null.");
- }
+ Utils.checkNull(area, "area");
scrollAreaToVisible(area.x, area.y, area.width, area.height);
}
@@ -1951,9 +1922,7 @@ public abstract class Component implemen
* @param immediate Whether or not the area needs immediate painting.
*/
public final void repaint(Bounds area, boolean immediate) {
- if (area == null) {
- throw new IllegalArgumentException("area is null.");
- }
+ Utils.checkNull(area, "area");
repaint(area.x, area.y, area.width, area.height, immediate);
}
@@ -2512,9 +2481,7 @@ public abstract class Component implemen
* @param styles A map containing the styles to apply.
*/
public void setStyles(Map<String, ?> styles) {
- if (styles == null) {
- throw new IllegalArgumentException("styles is null.");
- }
+ Utils.checkNull(styles, "styles");
for (String key : styles) {
getStyles().put(key, styles.get(key));
@@ -2528,9 +2495,7 @@ public abstract class Component implemen
* @throws SerializationException if the string doesn't conform to JSON standards.
*/
public void setStyles(String styles) throws SerializationException {
- if (styles == null) {
- throw new IllegalArgumentException("styles is null.");
- }
+ Utils.checkNull(styles, "styles");
setStyles(JSONSerializer.parseMap(styles));
}
@@ -2557,9 +2522,7 @@ public abstract class Component implemen
* @param styleName The name of an already loaded style to apply.
*/
public void setStyleName(String styleName) {
- if (styleName == null) {
- throw new IllegalArgumentException();
- }
+ Utils.checkNull(styleName, "styleName");
Map<String, ?> stylesLocal = namedStyles.get(styleName);
@@ -2576,9 +2539,7 @@ public abstract class Component implemen
* @param styleNames List of style names to apply to this component.
*/
public void setStyleNames(Sequence<String> styleNames) {
- if (styleNames == null) {
- throw new IllegalArgumentException();
- }
+ Utils.checkNull(styleNames, "styleNames");
for (int i = 0, n = styleNames.getLength(); i < n; i++) {
setStyleName(styleNames.get(i));
@@ -2591,9 +2552,7 @@ public abstract class Component implemen
* @param styleNames Comma-delimited list of style names to apply.
*/
public void setStyleNames(String styleNames) {
- if (styleNames == null) {
- throw new IllegalArgumentException();
- }
+ Utils.checkNull(styleNames, "styleNames");
for (String styleName : styleNames.split(",")) {
setStyleName(styleName.trim());
@@ -2601,7 +2560,6 @@ public abstract class Component implemen
}
/**
- * Returns the user data dictionary.
* @return The user data dictionary for this component.
*/
public UserDataDictionary getUserData() {
Modified: pivot/trunk/wtk/src/org/apache/pivot/wtk/Window.java
URL: http://svn.apache.org/viewvc/pivot/trunk/wtk/src/org/apache/pivot/wtk/Window.java?rev=1712423&r1=1712422&r2=1712423&view=diff
==============================================================================
--- pivot/trunk/wtk/src/org/apache/pivot/wtk/Window.java (original)
+++ pivot/trunk/wtk/src/org/apache/pivot/wtk/Window.java Tue Nov 3 21:44:48 2015
@@ -25,6 +25,7 @@ import org.apache.pivot.collections.Hash
import org.apache.pivot.collections.Sequence;
import org.apache.pivot.util.ImmutableIterator;
import org.apache.pivot.util.ListenerList;
+import org.apache.pivot.util.Utils;
import org.apache.pivot.util.Vote;
import org.apache.pivot.wtk.media.Image;
@@ -76,9 +77,7 @@ public class Window extends Container {
if (keyStroke != previousKeyStroke) {
if (window != null) {
- if (keyStroke == null) {
- throw new IllegalStateException();
- }
+ Utils.checkNull(keyStroke, "keyStroke");
if (window.actionMap.containsKey(keyStroke)) {
throw new IllegalArgumentException("A mapping for " + keyStroke
@@ -99,9 +98,7 @@ public class Window extends Container {
}
public void setKeyStroke(String keyStroke) {
- if (keyStroke == null) {
- throw new IllegalArgumentException("keyStroke is null.");
- }
+ Utils.checkNull(keyStroke, "keyStroke");
setKeyStroke(Keyboard.KeyStroke.decode(keyStroke));
}
@@ -115,9 +112,7 @@ public class Window extends Container {
if (action != previousAction) {
if (window != null) {
- if (action == null) {
- throw new IllegalStateException();
- }
+ Utils.checkNull(action, "action");
window.actionMap.put(keyStroke, action);
@@ -129,9 +124,7 @@ public class Window extends Container {
}
public void setAction(String actionID) {
- if (actionID == null) {
- throw new IllegalArgumentException("actionID is null");
- }
+ Utils.checkNull(actionID, "actionID");
Action actionLocal = Action.getNamedActions().get(actionID);
if (actionLocal == null) {
@@ -550,9 +543,7 @@ public class Window extends Container {
* window; <tt>false</tt>, otherwise.
*/
public boolean isOwner(Window window) {
- if (window == null) {
- throw new IllegalArgumentException("window is null.");
- }
+ Utils.checkNull(window, "window");
Window ownerLocal = window.getOwner();
@@ -599,9 +590,7 @@ public class Window extends Container {
* @throws IllegalArgumentException if the owner argument is {@code null}.
*/
public final void open(Window ownerArgument) {
- if (ownerArgument == null) {
- throw new IllegalArgumentException();
- }
+ Utils.checkNull(ownerArgument, "owner");
open(ownerArgument.getDisplay(), ownerArgument);
}
@@ -615,9 +604,7 @@ public class Window extends Container {
* has no owner.
*/
public void open(Display display, Window ownerArgument) {
- if (display == null) {
- throw new IllegalArgumentException("display is null.");
- }
+ Utils.checkNull(display, "display");
if (ownerArgument != null) {
if (!ownerArgument.isOpen()) {
@@ -784,9 +771,7 @@ public class Window extends Container {
* @param iconURL The location of the icon to set.
*/
public void setIcon(URL iconURL) {
- if (iconURL == null) {
- throw new IllegalArgumentException("iconURL is null.");
- }
+ Utils.checkNull(iconURL, "iconURL");
Image icon = Image.loadFromCache(iconURL);
@@ -802,14 +787,12 @@ public class Window extends Container {
* @see #setIcon(URL)
*/
public void setIcon(String iconName) {
- if (iconName == null) {
- throw new IllegalArgumentException("iconName is null.");
- }
+ Utils.checkNull(iconName, "iconName");
ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
URL url = classLoader.getResource(iconName.substring(1));
if (url == null) {
- throw new IllegalArgumentException("cannot find icon resource " + iconName);
+ throw new IllegalArgumentException("Cannot find icon resource " + iconName);
}
setIcon(url);
}
Re: Fwd: svn commit: r1712423 - in /pivot/trunk: core/src/org/apache/pivot/util/Utils.java
wtk/src/org/apache/pivot/wtk/Bounds.java wtk/src/org/apache/pivot/wtk/Component.java
wtk/src/org/apache/pivot/wtk/Window.java
Posted by Sandro Martini <sa...@gmail.com>.
Hi Roger,
> I guess my main motivation for doing this was seeing a lot of common
> code, that is, this pattern:
> if (blah == null) {
> throw new IllegalArgumentException("blah is null.")
> }
> and wanted to make some common code for it. Because lots of times the
> exception had no message, or it was slightly different. So, I also wanted
> to make the messages consistent, and the replacement code easy to write.
>
ok
> Because of that, I didn't look for any other alternatives (like the Java 7 Objects method).
no problem, I'm happy with all solutions :-) ...
> Although I did consider using an "assert"...
the problem of Java assert is that thay are by default not enabled
(and must be enabled with some flags), so don't know if really someone
uses them ... see:
http://docs.oracle.com/javase/7/docs/technotes/guides/language/assert.html
For example Groovy assertions are always enabled (so makes sense in
many real-world cases), and even in Scala (but could be removed with a
dedicated annotation, this is really great !!).
> But in the
> end, I just decided to make a small common method for this, mostly so that
> the kind of exception that is thrown (IllegalArgumentException) would stay
> the same (note that the Objects.requireNotNull throws a
> NullPointerException, which I don't like for these type of checks).
I agree with you :-)
I could post a comment in related issue, and update the code with
those methods ...
> I do like the idea of the method returning the object again, but that
> would require using generics, probably, or else you need a cast on every
> call, which is a bit of a pain. I would actually prefer an annotation
> ......... but that isn't supported in Java.
yes, maybe later or some test is useful and simple ...
And for the annotation see in a later release like 2.2 requiring
minimum Java 8 (with Java 8 things should be simpler even for
annotations).
> Also, I'd like to see the message generated from text resources, so it
> could be translated as required ... Which is a lot easier if all the checks
> are in one place.
Some time ago I wrote something like this (in a project at work) for
custom Exceptions, with messages from labels decaded in resource
bundles ... here we could do something like this even for this kind of
controlled exceptions, maybe in a dedicated issue.
Bye
Re: Fwd: svn commit: r1712423 - in /pivot/trunk:
core/src/org/apache/pivot/util/Utils.java
wtk/src/org/apache/pivot/wtk/Bounds.java
wtk/src/org/apache/pivot/wtk/Component.java
wtk/src/org/apache/pivot/wtk/Window.java
Posted by Roger and Beth Whitcomb <Ro...@rbwhitcomb.com>.
Hi all,
I guess my main motivation for doing this was seeing a lot of
common code, that is, this pattern:
if (blah == null) {
throw new IllegalArgumentException("blah is null.")
}
and wanted to make some common code for it. Because lots of times the
exception had no message, or it was slightly different. So, I also
wanted to make the messages consistent, and the replacement code easy to
write.
Because of that, I didn't look for any other alternatives (like the
Java 7 Objects method). Although I did consider using an "assert"...
But in the end, I just decided to make a small common method for this,
mostly so that the kind of exception that is thrown
(IllegalArgumentException) would stay the same (note that the
Objects.requireNotNull throws a NullPointerException, which I don't like
for these type of checks).
I do like the idea of the method returning the object again, but
that would require using generics, probably, or else you need a cast on
every call, which is a bit of a pain. I would actually prefer an
annotation ......... but that isn't supported in Java.
Also, I'd like to see the message generated from text resources, so
it could be translated as required ... Which is a lot easier if all the
checks are in one place.
Thanks,
~Roger
On 11/6/15 1:11 AM, Sandro Martini wrote:
> Hi all,
> just look at the commit (from Roger) for doing some cleanup, so not
> null checks for arguments are refactored in a utility method like
> this:
>
> Utils.checkNull(size, "size");
>
> I'm not against this (refactor common code is always good :-) ), but
> just I wonder if in trunk we could use a way for not null checks more
> aligned to Java 7 (minimum version required for 2.1.0), see here:
>
> http://docs.oracle.com/javase/7/docs/api/java/util/Objects.html#requireNonNull%28T,%20java.lang.String%29
>
> but in this (standard way) the generated Exception is a
> NullPointerException (doesn't look so good in that case, to me) and
> not an IllegalArgumentException.
> Some time ago I commetted some changes (under the PIVOT-799
> issue/improvement) like this:
>
> Objects.requireNonNull(t);
> Note that with it in some cases could be useful to have a simpler
> check and assignment to a variable in a single line, like:
> this.name = Objects.requireNonNull(nameArgument, "name cannot be null");
>
> Even those checks could be updated to the new common method, no
> problem, consistency is better. Maybe even an additional version that
> returns the value or throws the RuntimeException like in the example
> just seen.
>
>
> Note that we could even change the not null check implementation
> inside the general utility method later if/when needed ...
>
> Last, related issue should be
> [PIVOT-895](https://issues.apache.org/jira/browse/PIVOT-895), just for
> info.
>
> Generally speaking, our checks are mainly preconditions for methods
> (something like require in Scala) so IllegalArgumentException are
> good, maybe we could even add other methods to do a classical not null
> check that throws NullPointerException to use in other cases (like
> Java 7 methods, ou even using it in its implementation) ... so maybe
> the checkNull method could be renamed in requireNotNull and the new
> one as checkNotNull or similar ... and last maybe even a notEmpty for
> Strings (if used somewhere).
>
> What do you think ?
>
> Bye,
> Sandro
>
>
>
>
> ---------- Forwarded message ----------
> From: <rw...@apache.org>
> Date: 2015-11-03 22:44 GMT+01:00
> Subject: svn commit: r1712423 - in /pivot/trunk:
> core/src/org/apache/pivot/util/Utils.java
> wtk/src/org/apache/pivot/wtk/Bounds.java
> wtk/src/org/apache/pivot/wtk/Component.java
> wtk/src/org/apache/pivot/wtk/Window.java
> To: commits@pivot.apache.org
>
>
> Author: rwhitcomb
> Date: Tue Nov 3 21:44:48 2015
> New Revision: 1712423
>
> URL: http://svn.apache.org/viewvc?rev=1712423&view=rev
> Log:
> Code cleanup: Make a new method in the org.apache.pivot.utils.Utils
> class which does the
> common operation of checking an argument for null and throwing
> "IllegalArgumentException"
> if it is. This will simplify and standardize a lot of code, since
> this pattern appears
> so often, whenever objects are passed as arguments. And it will
> simplify getting an
> understandable message thrown out in every case. Before, there were
> many places that
> just threw the plain exception without a detail message, but this
> change will simplify
> the checking so there can always be a meaningful message.
>
> Change just a few files in the "wtk" area that do a lot of this
> checking already.
> Of course, there are many more places that could use this treatment...
>
>
> Modified:
> pivot/trunk/core/src/org/apache/pivot/util/Utils.java
> pivot/trunk/wtk/src/org/apache/pivot/wtk/Bounds.java
> pivot/trunk/wtk/src/org/apache/pivot/wtk/Component.java
> pivot/trunk/wtk/src/org/apache/pivot/wtk/Window.java
>
> Modified: pivot/trunk/core/src/org/apache/pivot/util/Utils.java
> URL: http://svn.apache.org/viewvc/pivot/trunk/core/src/org/apache/pivot/util/Utils.java?rev=1712423&r1=1712422&r2=1712423&view=diff
> ==============================================================================
> --- pivot/trunk/core/src/org/apache/pivot/util/Utils.java (original)
> +++ pivot/trunk/core/src/org/apache/pivot/util/Utils.java Tue Nov 3
> 21:44:48 2015
> @@ -38,4 +38,26 @@ public class Utils {
> return s1.equals(s2);
> }
>
> + /**
> + * Check if the input argument is {@code null} and throw an
> + * {@link IllegalArgumentException} if so, with an optional
> + * message derived from the given string.
> + *
> + * @param value The argument value to check for {@code null}.
> + * @param description A description for the value used to
> + * construct a message like {@code "xxx must not be null."}. Can be
> + * {@code null} in which case a plain {@link IllegalArgumentException}
> + * is thrown without any detail message.
> + * @throws IllegalArgumentException if the value is {@code null}.
> + */
> + public static void checkNull(Object value, String description) {
> + if (value == null) {
> + if (description == null) {
> + throw new IllegalArgumentException();
> + } else {
> + throw new IllegalArgumentException(description + "
> must not be null.");
> + }
> + }
> + }
> +
> }
>
> Modified: pivot/trunk/wtk/src/org/apache/pivot/wtk/Bounds.java
> URL: http://svn.apache.org/viewvc/pivot/trunk/wtk/src/org/apache/pivot/wtk/Bounds.java?rev=1712423&r1=1712422&r2=1712423&view=diff
> ==============================================================================
> --- pivot/trunk/wtk/src/org/apache/pivot/wtk/Bounds.java (original)
> +++ pivot/trunk/wtk/src/org/apache/pivot/wtk/Bounds.java Tue Nov 3
> 21:44:48 2015
> @@ -21,6 +21,7 @@ import java.io.Serializable;
> import org.apache.pivot.collections.Dictionary;
> import org.apache.pivot.json.JSONSerializer;
> import org.apache.pivot.serialization.SerializationException;
> +import org.apache.pivot.util.Utils;
>
> /**
> * Class representing the bounds of an object.
> @@ -46,13 +47,8 @@ public final class Bounds implements Ser
> }
>
> public Bounds(Point origin, Dimensions size) {
> - if (origin == null) {
> - throw new IllegalArgumentException("origin is null.");
> - }
> -
> - if (size == null) {
> - throw new IllegalArgumentException("size is null.");
> - }
> + Utils.checkNull(origin, "origin");
> + Utils.checkNull(size, "size");
>
> x = origin.x;
> y = origin.y;
> @@ -61,9 +57,7 @@ public final class Bounds implements Ser
> }
>
> public Bounds(Bounds bounds) {
> - if (bounds == null) {
> - throw new IllegalArgumentException("bounds is null.");
> - }
> + Utils.checkNull(bounds, "bounds");
>
> x = bounds.x;
> y = bounds.y;
> @@ -72,9 +66,7 @@ public final class Bounds implements Ser
> }
>
> public Bounds(Dictionary<String, ?> bounds) {
> - if (bounds == null) {
> - throw new IllegalArgumentException("bounds is null.");
> - }
> + Utils.checkNull(bounds, "bounds");
>
> if (bounds.containsKey(X_KEY)) {
> x = ((Integer) bounds.get(X_KEY)).intValue();
> @@ -102,9 +94,7 @@ public final class Bounds implements Ser
> }
>
> public Bounds(java.awt.Rectangle rectangle) {
> - if (rectangle == null) {
> - throw new IllegalArgumentException("rectangle is null.");
> - }
> + Utils.checkNull(rectangle, "rectangle");
>
> x = rectangle.x;
> y = rectangle.y;
> @@ -160,9 +150,7 @@ public final class Bounds implements Ser
> }
>
> public boolean contains(Point point) {
> - if (point == null) {
> - throw new IllegalArgumentException("point is null");
> - }
> + Utils.checkNull(point, "point");
>
> return contains(point.x, point.y);
> }
> @@ -173,9 +161,7 @@ public final class Bounds implements Ser
> }
>
> public boolean contains(Bounds bounds) {
> - if (bounds == null) {
> - throw new IllegalArgumentException("bounds is null");
> - }
> + Utils.checkNull(bounds, "bounds");
>
> return contains(bounds.x, bounds.y, bounds.width, bounds.height);
> }
> @@ -187,9 +173,7 @@ public final class Bounds implements Ser
> }
>
> public boolean intersects(Bounds bounds) {
> - if (bounds == null) {
> - throw new IllegalArgumentException("bounds is null");
> - }
> + Utils.checkNull(bounds, "bounds");
>
> return intersects(bounds.x, bounds.y, bounds.width, bounds.height);
> }
> @@ -236,14 +220,12 @@ public final class Bounds implements Ser
> return getClass().getName() + " [" + x + "," + y + ";" +
> width + "x" + height + "]";
> }
>
> - public static Bounds decode(String value) {
> - if (value == null) {
> - throw new IllegalArgumentException();
> - }
> + public static Bounds decode(String boundsValue) {
> + Utils.checkNull(boundsValue, "boundsValue");
>
> Bounds bounds;
> try {
> - bounds = new Bounds(JSONSerializer.parseMap(value));
> + bounds = new Bounds(JSONSerializer.parseMap(boundsValue));
> } catch (SerializationException exception) {
> throw new IllegalArgumentException(exception);
> }
>
> Modified: pivot/trunk/wtk/src/org/apache/pivot/wtk/Component.java
> URL: http://svn.apache.org/viewvc/pivot/trunk/wtk/src/org/apache/pivot/wtk/Component.java?rev=1712423&r1=1712422&r2=1712423&view=diff
> ==============================================================================
> --- pivot/trunk/wtk/src/org/apache/pivot/wtk/Component.java (original)
> +++ pivot/trunk/wtk/src/org/apache/pivot/wtk/Component.java Tue Nov 3
> 21:44:48 2015
> @@ -35,6 +35,7 @@ import org.apache.pivot.json.JSONSeriali
> import org.apache.pivot.serialization.SerializationException;
> import org.apache.pivot.util.ImmutableIterator;
> import org.apache.pivot.util.ListenerList;
> +import org.apache.pivot.util.Utils;
> import org.apache.pivot.wtk.effects.Decorator;
>
> /**
> @@ -177,9 +178,7 @@ public abstract class Component implemen
>
> @Override
> public void insert(Decorator decorator, int index) {
> - if (decorator == null) {
> - throw new IllegalArgumentException("decorator is null");
> - }
> + Utils.checkNull(decorator, "decorator");
>
> // Repaint the the component's previous decorated region
> if (parent != null) {
> @@ -198,9 +197,7 @@ public abstract class Component implemen
>
> @Override
> public Decorator update(int index, Decorator decorator) {
> - if (decorator == null) {
> - throw new IllegalArgumentException("decorator is null.");
> - }
> + Utils.checkNull(decorator, "decorator");
>
> // Repaint the the component's previous decorated region
> if (parent != null) {
> @@ -744,9 +741,7 @@ public abstract class Component implemen
> */
> @SuppressWarnings("unchecked")
> protected void setSkin(Skin skin) {
> - if (skin == null) {
> - throw new IllegalArgumentException("skin is null.");
> - }
> + Utils.checkNull(skin, "skin");
>
> if (this.skin != null) {
> throw new IllegalStateException("Skin is already installed.");
> @@ -868,9 +863,7 @@ public abstract class Component implemen
>
> @SuppressWarnings("unchecked")
> public Container getAncestor(String ancestorTypeName) throws
> ClassNotFoundException {
> - if (ancestorTypeName == null) {
> - throw new IllegalArgumentException();
> - }
> + Utils.checkNull(ancestorTypeName, "ancestorTypeName");
>
> return getAncestor((Class<? extends Container>)
> Class.forName(ancestorTypeName));
> }
> @@ -898,9 +891,7 @@ public abstract class Component implemen
> }
>
> public final void setSize(Dimensions size) {
> - if (size == null) {
> - throw new IllegalArgumentException("size is null.");
> - }
> + Utils.checkNull(size, "size");
>
> setSize(size.width, size.height);
> }
> @@ -1105,9 +1096,7 @@ public abstract class Component implemen
> }
>
> public final void setPreferredSize(Dimensions preferredSize) {
> - if (preferredSize == null) {
> - throw new IllegalArgumentException("preferredSize is null.");
> - }
> + Utils.checkNull(preferredSize, "preferredSize");
>
> setPreferredSize(preferredSize.width, preferredSize.height);
> }
> @@ -1232,9 +1221,7 @@ public abstract class Component implemen
> * @param widthLimits The new width limits (min and max).
> */
> public final void setWidthLimits(Limits widthLimits) {
> - if (widthLimits == null) {
> - throw new IllegalArgumentException("widthLimits is null.");
> - }
> + Utils.checkNull(widthLimits, "widthLimits");
>
> setWidthLimits(widthLimits.minimum, widthLimits.maximum);
> }
> @@ -1316,9 +1303,7 @@ public abstract class Component implemen
> * @param heightLimits The new height limits (min and max).
> */
> public final void setHeightLimits(Limits heightLimits) {
> - if (heightLimits == null) {
> - throw new IllegalArgumentException("heightLimits is null.");
> - }
> + Utils.checkNull(heightLimits, "heightLimits");
>
> setHeightLimits(heightLimits.minimum, heightLimits.maximum);
> }
> @@ -1415,9 +1400,7 @@ public abstract class Component implemen
> * @see #setLocation(int, int)
> */
> public final void setLocation(Point location) {
> - if (location == null) {
> - throw new IllegalArgumentException("location cannot be null.");
> - }
> + Utils.checkNull(location, "location");
>
> setLocation(location.x, location.y);
> }
> @@ -1568,9 +1551,7 @@ public abstract class Component implemen
> * the component is not a descendant of the specified ancestor.
> */
> public Point mapPointToAncestor(final Container ancestor, int
> xArgument, int yArgument) {
> - if (ancestor == null) {
> - throw new IllegalArgumentException("ancestor is null");
> - }
> + Utils.checkNull(ancestor, "ancestor");
>
> int xArgumentMutable = xArgument;
> int yArgumentMutable = yArgument;
> @@ -1603,9 +1584,7 @@ public abstract class Component implemen
> * the component is not a descendant of the specified ancestor.
> */
> public Point mapPointToAncestor(Container ancestor, Point location) {
> - if (location == null) {
> - throw new IllegalArgumentException();
> - }
> + Utils.checkNull(location, "location");
>
> return mapPointToAncestor(ancestor, location.x, location.y);
> }
> @@ -1621,9 +1600,7 @@ public abstract class Component implemen
> * the component is not a descendant of the specified ancestor.
> */
> public Point mapPointFromAncestor(final Container ancestor, int
> xArgument, int yArgument) {
> - if (ancestor == null) {
> - throw new IllegalArgumentException("ancestor is null");
> - }
> + Utils.checkNull(ancestor, "ancestor");
>
> int xArgumentMutable = xArgument;
> int yArgumentMutable = yArgument;
> @@ -1646,9 +1623,7 @@ public abstract class Component implemen
> }
>
> public Point mapPointFromAncestor(Container ancestor, Point location) {
> - if (location == null) {
> - throw new IllegalArgumentException();
> - }
> + Utils.checkNull(location, "location");
>
> return mapPointFromAncestor(ancestor, location.x, location.y);
> }
> @@ -1694,9 +1669,7 @@ public abstract class Component implemen
> * part of the component hierarchy.
> */
> public Bounds getVisibleArea(Bounds area) {
> - if (area == null) {
> - throw new IllegalArgumentException("area is null.");
> - }
> + Utils.checkNull(area, "area");
>
> return getVisibleArea(area.x, area.y, area.width, area.height);
> }
> @@ -1768,9 +1741,7 @@ public abstract class Component implemen
> * @param area The area to be made visible.
> */
> public void scrollAreaToVisible(Bounds area) {
> - if (area == null) {
> - throw new IllegalArgumentException("area is null.");
> - }
> + Utils.checkNull(area, "area");
>
> scrollAreaToVisible(area.x, area.y, area.width, area.height);
> }
> @@ -1951,9 +1922,7 @@ public abstract class Component implemen
> * @param immediate Whether or not the area needs immediate painting.
> */
> public final void repaint(Bounds area, boolean immediate) {
> - if (area == null) {
> - throw new IllegalArgumentException("area is null.");
> - }
> + Utils.checkNull(area, "area");
>
> repaint(area.x, area.y, area.width, area.height, immediate);
> }
> @@ -2512,9 +2481,7 @@ public abstract class Component implemen
> * @param styles A map containing the styles to apply.
> */
> public void setStyles(Map<String, ?> styles) {
> - if (styles == null) {
> - throw new IllegalArgumentException("styles is null.");
> - }
> + Utils.checkNull(styles, "styles");
>
> for (String key : styles) {
> getStyles().put(key, styles.get(key));
> @@ -2528,9 +2495,7 @@ public abstract class Component implemen
> * @throws SerializationException if the string doesn't conform
> to JSON standards.
> */
> public void setStyles(String styles) throws SerializationException {
> - if (styles == null) {
> - throw new IllegalArgumentException("styles is null.");
> - }
> + Utils.checkNull(styles, "styles");
>
> setStyles(JSONSerializer.parseMap(styles));
> }
> @@ -2557,9 +2522,7 @@ public abstract class Component implemen
> * @param styleName The name of an already loaded style to apply.
> */
> public void setStyleName(String styleName) {
> - if (styleName == null) {
> - throw new IllegalArgumentException();
> - }
> + Utils.checkNull(styleName, "styleName");
>
> Map<String, ?> stylesLocal = namedStyles.get(styleName);
>
> @@ -2576,9 +2539,7 @@ public abstract class Component implemen
> * @param styleNames List of style names to apply to this component.
> */
> public void setStyleNames(Sequence<String> styleNames) {
> - if (styleNames == null) {
> - throw new IllegalArgumentException();
> - }
> + Utils.checkNull(styleNames, "styleNames");
>
> for (int i = 0, n = styleNames.getLength(); i < n; i++) {
> setStyleName(styleNames.get(i));
> @@ -2591,9 +2552,7 @@ public abstract class Component implemen
> * @param styleNames Comma-delimited list of style names to apply.
> */
> public void setStyleNames(String styleNames) {
> - if (styleNames == null) {
> - throw new IllegalArgumentException();
> - }
> + Utils.checkNull(styleNames, "styleNames");
>
> for (String styleName : styleNames.split(",")) {
> setStyleName(styleName.trim());
> @@ -2601,7 +2560,6 @@ public abstract class Component implemen
> }
>
> /**
> - * Returns the user data dictionary.
> * @return The user data dictionary for this component.
> */
> public UserDataDictionary getUserData() {
>
> Modified: pivot/trunk/wtk/src/org/apache/pivot/wtk/Window.java
> URL: http://svn.apache.org/viewvc/pivot/trunk/wtk/src/org/apache/pivot/wtk/Window.java?rev=1712423&r1=1712422&r2=1712423&view=diff
> ==============================================================================
> --- pivot/trunk/wtk/src/org/apache/pivot/wtk/Window.java (original)
> +++ pivot/trunk/wtk/src/org/apache/pivot/wtk/Window.java Tue Nov 3
> 21:44:48 2015
> @@ -25,6 +25,7 @@ import org.apache.pivot.collections.Hash
> import org.apache.pivot.collections.Sequence;
> import org.apache.pivot.util.ImmutableIterator;
> import org.apache.pivot.util.ListenerList;
> +import org.apache.pivot.util.Utils;
> import org.apache.pivot.util.Vote;
> import org.apache.pivot.wtk.media.Image;
>
> @@ -76,9 +77,7 @@ public class Window extends Container {
>
> if (keyStroke != previousKeyStroke) {
> if (window != null) {
> - if (keyStroke == null) {
> - throw new IllegalStateException();
> - }
> + Utils.checkNull(keyStroke, "keyStroke");
>
> if (window.actionMap.containsKey(keyStroke)) {
> throw new IllegalArgumentException("A mapping
> for " + keyStroke
> @@ -99,9 +98,7 @@ public class Window extends Container {
> }
>
> public void setKeyStroke(String keyStroke) {
> - if (keyStroke == null) {
> - throw new IllegalArgumentException("keyStroke is null.");
> - }
> + Utils.checkNull(keyStroke, "keyStroke");
>
> setKeyStroke(Keyboard.KeyStroke.decode(keyStroke));
> }
> @@ -115,9 +112,7 @@ public class Window extends Container {
>
> if (action != previousAction) {
> if (window != null) {
> - if (action == null) {
> - throw new IllegalStateException();
> - }
> + Utils.checkNull(action, "action");
>
> window.actionMap.put(keyStroke, action);
>
> @@ -129,9 +124,7 @@ public class Window extends Container {
> }
>
> public void setAction(String actionID) {
> - if (actionID == null) {
> - throw new IllegalArgumentException("actionID is null");
> - }
> + Utils.checkNull(actionID, "actionID");
>
> Action actionLocal = Action.getNamedActions().get(actionID);
> if (actionLocal == null) {
> @@ -550,9 +543,7 @@ public class Window extends Container {
> * window; <tt>false</tt>, otherwise.
> */
> public boolean isOwner(Window window) {
> - if (window == null) {
> - throw new IllegalArgumentException("window is null.");
> - }
> + Utils.checkNull(window, "window");
>
> Window ownerLocal = window.getOwner();
>
> @@ -599,9 +590,7 @@ public class Window extends Container {
> * @throws IllegalArgumentException if the owner argument is {@code null}.
> */
> public final void open(Window ownerArgument) {
> - if (ownerArgument == null) {
> - throw new IllegalArgumentException();
> - }
> + Utils.checkNull(ownerArgument, "owner");
>
> open(ownerArgument.getDisplay(), ownerArgument);
> }
> @@ -615,9 +604,7 @@ public class Window extends Container {
> * has no owner.
> */
> public void open(Display display, Window ownerArgument) {
> - if (display == null) {
> - throw new IllegalArgumentException("display is null.");
> - }
> + Utils.checkNull(display, "display");
>
> if (ownerArgument != null) {
> if (!ownerArgument.isOpen()) {
> @@ -784,9 +771,7 @@ public class Window extends Container {
> * @param iconURL The location of the icon to set.
> */
> public void setIcon(URL iconURL) {
> - if (iconURL == null) {
> - throw new IllegalArgumentException("iconURL is null.");
> - }
> + Utils.checkNull(iconURL, "iconURL");
>
> Image icon = Image.loadFromCache(iconURL);
>
> @@ -802,14 +787,12 @@ public class Window extends Container {
> * @see #setIcon(URL)
> */
> public void setIcon(String iconName) {
> - if (iconName == null) {
> - throw new IllegalArgumentException("iconName is null.");
> - }
> + Utils.checkNull(iconName, "iconName");
>
> ClassLoader classLoader =
> Thread.currentThread().getContextClassLoader();
> URL url = classLoader.getResource(iconName.substring(1));
> if (url == null) {
> - throw new IllegalArgumentException("cannot find icon
> resource " + iconName);
> + throw new IllegalArgumentException("Cannot find icon
> resource " + iconName);
> }
> setIcon(url);
> }
>
>
Fwd: svn commit: r1712423 - in /pivot/trunk: core/src/org/apache/pivot/util/Utils.java
wtk/src/org/apache/pivot/wtk/Bounds.java wtk/src/org/apache/pivot/wtk/Component.java
wtk/src/org/apache/pivot/wtk/Window.java
Posted by Sandro Martini <sa...@gmail.com>.
Hi all,
just look at the commit (from Roger) for doing some cleanup, so not
null checks for arguments are refactored in a utility method like
this:
Utils.checkNull(size, "size");
I'm not against this (refactor common code is always good :-) ), but
just I wonder if in trunk we could use a way for not null checks more
aligned to Java 7 (minimum version required for 2.1.0), see here:
http://docs.oracle.com/javase/7/docs/api/java/util/Objects.html#requireNonNull%28T,%20java.lang.String%29
but in this (standard way) the generated Exception is a
NullPointerException (doesn't look so good in that case, to me) and
not an IllegalArgumentException.
Some time ago I commetted some changes (under the PIVOT-799
issue/improvement) like this:
Objects.requireNonNull(t);
Note that with it in some cases could be useful to have a simpler
check and assignment to a variable in a single line, like:
this.name = Objects.requireNonNull(nameArgument, "name cannot be null");
Even those checks could be updated to the new common method, no
problem, consistency is better. Maybe even an additional version that
returns the value or throws the RuntimeException like in the example
just seen.
Note that we could even change the not null check implementation
inside the general utility method later if/when needed ...
Last, related issue should be
[PIVOT-895](https://issues.apache.org/jira/browse/PIVOT-895), just for
info.
Generally speaking, our checks are mainly preconditions for methods
(something like require in Scala) so IllegalArgumentException are
good, maybe we could even add other methods to do a classical not null
check that throws NullPointerException to use in other cases (like
Java 7 methods, ou even using it in its implementation) ... so maybe
the checkNull method could be renamed in requireNotNull and the new
one as checkNotNull or similar ... and last maybe even a notEmpty for
Strings (if used somewhere).
What do you think ?
Bye,
Sandro
---------- Forwarded message ----------
From: <rw...@apache.org>
Date: 2015-11-03 22:44 GMT+01:00
Subject: svn commit: r1712423 - in /pivot/trunk:
core/src/org/apache/pivot/util/Utils.java
wtk/src/org/apache/pivot/wtk/Bounds.java
wtk/src/org/apache/pivot/wtk/Component.java
wtk/src/org/apache/pivot/wtk/Window.java
To: commits@pivot.apache.org
Author: rwhitcomb
Date: Tue Nov 3 21:44:48 2015
New Revision: 1712423
URL: http://svn.apache.org/viewvc?rev=1712423&view=rev
Log:
Code cleanup: Make a new method in the org.apache.pivot.utils.Utils
class which does the
common operation of checking an argument for null and throwing
"IllegalArgumentException"
if it is. This will simplify and standardize a lot of code, since
this pattern appears
so often, whenever objects are passed as arguments. And it will
simplify getting an
understandable message thrown out in every case. Before, there were
many places that
just threw the plain exception without a detail message, but this
change will simplify
the checking so there can always be a meaningful message.
Change just a few files in the "wtk" area that do a lot of this
checking already.
Of course, there are many more places that could use this treatment...
Modified:
pivot/trunk/core/src/org/apache/pivot/util/Utils.java
pivot/trunk/wtk/src/org/apache/pivot/wtk/Bounds.java
pivot/trunk/wtk/src/org/apache/pivot/wtk/Component.java
pivot/trunk/wtk/src/org/apache/pivot/wtk/Window.java
Modified: pivot/trunk/core/src/org/apache/pivot/util/Utils.java
URL: http://svn.apache.org/viewvc/pivot/trunk/core/src/org/apache/pivot/util/Utils.java?rev=1712423&r1=1712422&r2=1712423&view=diff
==============================================================================
--- pivot/trunk/core/src/org/apache/pivot/util/Utils.java (original)
+++ pivot/trunk/core/src/org/apache/pivot/util/Utils.java Tue Nov 3
21:44:48 2015
@@ -38,4 +38,26 @@ public class Utils {
return s1.equals(s2);
}
+ /**
+ * Check if the input argument is {@code null} and throw an
+ * {@link IllegalArgumentException} if so, with an optional
+ * message derived from the given string.
+ *
+ * @param value The argument value to check for {@code null}.
+ * @param description A description for the value used to
+ * construct a message like {@code "xxx must not be null."}. Can be
+ * {@code null} in which case a plain {@link IllegalArgumentException}
+ * is thrown without any detail message.
+ * @throws IllegalArgumentException if the value is {@code null}.
+ */
+ public static void checkNull(Object value, String description) {
+ if (value == null) {
+ if (description == null) {
+ throw new IllegalArgumentException();
+ } else {
+ throw new IllegalArgumentException(description + "
must not be null.");
+ }
+ }
+ }
+
}
Modified: pivot/trunk/wtk/src/org/apache/pivot/wtk/Bounds.java
URL: http://svn.apache.org/viewvc/pivot/trunk/wtk/src/org/apache/pivot/wtk/Bounds.java?rev=1712423&r1=1712422&r2=1712423&view=diff
==============================================================================
--- pivot/trunk/wtk/src/org/apache/pivot/wtk/Bounds.java (original)
+++ pivot/trunk/wtk/src/org/apache/pivot/wtk/Bounds.java Tue Nov 3
21:44:48 2015
@@ -21,6 +21,7 @@ import java.io.Serializable;
import org.apache.pivot.collections.Dictionary;
import org.apache.pivot.json.JSONSerializer;
import org.apache.pivot.serialization.SerializationException;
+import org.apache.pivot.util.Utils;
/**
* Class representing the bounds of an object.
@@ -46,13 +47,8 @@ public final class Bounds implements Ser
}
public Bounds(Point origin, Dimensions size) {
- if (origin == null) {
- throw new IllegalArgumentException("origin is null.");
- }
-
- if (size == null) {
- throw new IllegalArgumentException("size is null.");
- }
+ Utils.checkNull(origin, "origin");
+ Utils.checkNull(size, "size");
x = origin.x;
y = origin.y;
@@ -61,9 +57,7 @@ public final class Bounds implements Ser
}
public Bounds(Bounds bounds) {
- if (bounds == null) {
- throw new IllegalArgumentException("bounds is null.");
- }
+ Utils.checkNull(bounds, "bounds");
x = bounds.x;
y = bounds.y;
@@ -72,9 +66,7 @@ public final class Bounds implements Ser
}
public Bounds(Dictionary<String, ?> bounds) {
- if (bounds == null) {
- throw new IllegalArgumentException("bounds is null.");
- }
+ Utils.checkNull(bounds, "bounds");
if (bounds.containsKey(X_KEY)) {
x = ((Integer) bounds.get(X_KEY)).intValue();
@@ -102,9 +94,7 @@ public final class Bounds implements Ser
}
public Bounds(java.awt.Rectangle rectangle) {
- if (rectangle == null) {
- throw new IllegalArgumentException("rectangle is null.");
- }
+ Utils.checkNull(rectangle, "rectangle");
x = rectangle.x;
y = rectangle.y;
@@ -160,9 +150,7 @@ public final class Bounds implements Ser
}
public boolean contains(Point point) {
- if (point == null) {
- throw new IllegalArgumentException("point is null");
- }
+ Utils.checkNull(point, "point");
return contains(point.x, point.y);
}
@@ -173,9 +161,7 @@ public final class Bounds implements Ser
}
public boolean contains(Bounds bounds) {
- if (bounds == null) {
- throw new IllegalArgumentException("bounds is null");
- }
+ Utils.checkNull(bounds, "bounds");
return contains(bounds.x, bounds.y, bounds.width, bounds.height);
}
@@ -187,9 +173,7 @@ public final class Bounds implements Ser
}
public boolean intersects(Bounds bounds) {
- if (bounds == null) {
- throw new IllegalArgumentException("bounds is null");
- }
+ Utils.checkNull(bounds, "bounds");
return intersects(bounds.x, bounds.y, bounds.width, bounds.height);
}
@@ -236,14 +220,12 @@ public final class Bounds implements Ser
return getClass().getName() + " [" + x + "," + y + ";" +
width + "x" + height + "]";
}
- public static Bounds decode(String value) {
- if (value == null) {
- throw new IllegalArgumentException();
- }
+ public static Bounds decode(String boundsValue) {
+ Utils.checkNull(boundsValue, "boundsValue");
Bounds bounds;
try {
- bounds = new Bounds(JSONSerializer.parseMap(value));
+ bounds = new Bounds(JSONSerializer.parseMap(boundsValue));
} catch (SerializationException exception) {
throw new IllegalArgumentException(exception);
}
Modified: pivot/trunk/wtk/src/org/apache/pivot/wtk/Component.java
URL: http://svn.apache.org/viewvc/pivot/trunk/wtk/src/org/apache/pivot/wtk/Component.java?rev=1712423&r1=1712422&r2=1712423&view=diff
==============================================================================
--- pivot/trunk/wtk/src/org/apache/pivot/wtk/Component.java (original)
+++ pivot/trunk/wtk/src/org/apache/pivot/wtk/Component.java Tue Nov 3
21:44:48 2015
@@ -35,6 +35,7 @@ import org.apache.pivot.json.JSONSeriali
import org.apache.pivot.serialization.SerializationException;
import org.apache.pivot.util.ImmutableIterator;
import org.apache.pivot.util.ListenerList;
+import org.apache.pivot.util.Utils;
import org.apache.pivot.wtk.effects.Decorator;
/**
@@ -177,9 +178,7 @@ public abstract class Component implemen
@Override
public void insert(Decorator decorator, int index) {
- if (decorator == null) {
- throw new IllegalArgumentException("decorator is null");
- }
+ Utils.checkNull(decorator, "decorator");
// Repaint the the component's previous decorated region
if (parent != null) {
@@ -198,9 +197,7 @@ public abstract class Component implemen
@Override
public Decorator update(int index, Decorator decorator) {
- if (decorator == null) {
- throw new IllegalArgumentException("decorator is null.");
- }
+ Utils.checkNull(decorator, "decorator");
// Repaint the the component's previous decorated region
if (parent != null) {
@@ -744,9 +741,7 @@ public abstract class Component implemen
*/
@SuppressWarnings("unchecked")
protected void setSkin(Skin skin) {
- if (skin == null) {
- throw new IllegalArgumentException("skin is null.");
- }
+ Utils.checkNull(skin, "skin");
if (this.skin != null) {
throw new IllegalStateException("Skin is already installed.");
@@ -868,9 +863,7 @@ public abstract class Component implemen
@SuppressWarnings("unchecked")
public Container getAncestor(String ancestorTypeName) throws
ClassNotFoundException {
- if (ancestorTypeName == null) {
- throw new IllegalArgumentException();
- }
+ Utils.checkNull(ancestorTypeName, "ancestorTypeName");
return getAncestor((Class<? extends Container>)
Class.forName(ancestorTypeName));
}
@@ -898,9 +891,7 @@ public abstract class Component implemen
}
public final void setSize(Dimensions size) {
- if (size == null) {
- throw new IllegalArgumentException("size is null.");
- }
+ Utils.checkNull(size, "size");
setSize(size.width, size.height);
}
@@ -1105,9 +1096,7 @@ public abstract class Component implemen
}
public final void setPreferredSize(Dimensions preferredSize) {
- if (preferredSize == null) {
- throw new IllegalArgumentException("preferredSize is null.");
- }
+ Utils.checkNull(preferredSize, "preferredSize");
setPreferredSize(preferredSize.width, preferredSize.height);
}
@@ -1232,9 +1221,7 @@ public abstract class Component implemen
* @param widthLimits The new width limits (min and max).
*/
public final void setWidthLimits(Limits widthLimits) {
- if (widthLimits == null) {
- throw new IllegalArgumentException("widthLimits is null.");
- }
+ Utils.checkNull(widthLimits, "widthLimits");
setWidthLimits(widthLimits.minimum, widthLimits.maximum);
}
@@ -1316,9 +1303,7 @@ public abstract class Component implemen
* @param heightLimits The new height limits (min and max).
*/
public final void setHeightLimits(Limits heightLimits) {
- if (heightLimits == null) {
- throw new IllegalArgumentException("heightLimits is null.");
- }
+ Utils.checkNull(heightLimits, "heightLimits");
setHeightLimits(heightLimits.minimum, heightLimits.maximum);
}
@@ -1415,9 +1400,7 @@ public abstract class Component implemen
* @see #setLocation(int, int)
*/
public final void setLocation(Point location) {
- if (location == null) {
- throw new IllegalArgumentException("location cannot be null.");
- }
+ Utils.checkNull(location, "location");
setLocation(location.x, location.y);
}
@@ -1568,9 +1551,7 @@ public abstract class Component implemen
* the component is not a descendant of the specified ancestor.
*/
public Point mapPointToAncestor(final Container ancestor, int
xArgument, int yArgument) {
- if (ancestor == null) {
- throw new IllegalArgumentException("ancestor is null");
- }
+ Utils.checkNull(ancestor, "ancestor");
int xArgumentMutable = xArgument;
int yArgumentMutable = yArgument;
@@ -1603,9 +1584,7 @@ public abstract class Component implemen
* the component is not a descendant of the specified ancestor.
*/
public Point mapPointToAncestor(Container ancestor, Point location) {
- if (location == null) {
- throw new IllegalArgumentException();
- }
+ Utils.checkNull(location, "location");
return mapPointToAncestor(ancestor, location.x, location.y);
}
@@ -1621,9 +1600,7 @@ public abstract class Component implemen
* the component is not a descendant of the specified ancestor.
*/
public Point mapPointFromAncestor(final Container ancestor, int
xArgument, int yArgument) {
- if (ancestor == null) {
- throw new IllegalArgumentException("ancestor is null");
- }
+ Utils.checkNull(ancestor, "ancestor");
int xArgumentMutable = xArgument;
int yArgumentMutable = yArgument;
@@ -1646,9 +1623,7 @@ public abstract class Component implemen
}
public Point mapPointFromAncestor(Container ancestor, Point location) {
- if (location == null) {
- throw new IllegalArgumentException();
- }
+ Utils.checkNull(location, "location");
return mapPointFromAncestor(ancestor, location.x, location.y);
}
@@ -1694,9 +1669,7 @@ public abstract class Component implemen
* part of the component hierarchy.
*/
public Bounds getVisibleArea(Bounds area) {
- if (area == null) {
- throw new IllegalArgumentException("area is null.");
- }
+ Utils.checkNull(area, "area");
return getVisibleArea(area.x, area.y, area.width, area.height);
}
@@ -1768,9 +1741,7 @@ public abstract class Component implemen
* @param area The area to be made visible.
*/
public void scrollAreaToVisible(Bounds area) {
- if (area == null) {
- throw new IllegalArgumentException("area is null.");
- }
+ Utils.checkNull(area, "area");
scrollAreaToVisible(area.x, area.y, area.width, area.height);
}
@@ -1951,9 +1922,7 @@ public abstract class Component implemen
* @param immediate Whether or not the area needs immediate painting.
*/
public final void repaint(Bounds area, boolean immediate) {
- if (area == null) {
- throw new IllegalArgumentException("area is null.");
- }
+ Utils.checkNull(area, "area");
repaint(area.x, area.y, area.width, area.height, immediate);
}
@@ -2512,9 +2481,7 @@ public abstract class Component implemen
* @param styles A map containing the styles to apply.
*/
public void setStyles(Map<String, ?> styles) {
- if (styles == null) {
- throw new IllegalArgumentException("styles is null.");
- }
+ Utils.checkNull(styles, "styles");
for (String key : styles) {
getStyles().put(key, styles.get(key));
@@ -2528,9 +2495,7 @@ public abstract class Component implemen
* @throws SerializationException if the string doesn't conform
to JSON standards.
*/
public void setStyles(String styles) throws SerializationException {
- if (styles == null) {
- throw new IllegalArgumentException("styles is null.");
- }
+ Utils.checkNull(styles, "styles");
setStyles(JSONSerializer.parseMap(styles));
}
@@ -2557,9 +2522,7 @@ public abstract class Component implemen
* @param styleName The name of an already loaded style to apply.
*/
public void setStyleName(String styleName) {
- if (styleName == null) {
- throw new IllegalArgumentException();
- }
+ Utils.checkNull(styleName, "styleName");
Map<String, ?> stylesLocal = namedStyles.get(styleName);
@@ -2576,9 +2539,7 @@ public abstract class Component implemen
* @param styleNames List of style names to apply to this component.
*/
public void setStyleNames(Sequence<String> styleNames) {
- if (styleNames == null) {
- throw new IllegalArgumentException();
- }
+ Utils.checkNull(styleNames, "styleNames");
for (int i = 0, n = styleNames.getLength(); i < n; i++) {
setStyleName(styleNames.get(i));
@@ -2591,9 +2552,7 @@ public abstract class Component implemen
* @param styleNames Comma-delimited list of style names to apply.
*/
public void setStyleNames(String styleNames) {
- if (styleNames == null) {
- throw new IllegalArgumentException();
- }
+ Utils.checkNull(styleNames, "styleNames");
for (String styleName : styleNames.split(",")) {
setStyleName(styleName.trim());
@@ -2601,7 +2560,6 @@ public abstract class Component implemen
}
/**
- * Returns the user data dictionary.
* @return The user data dictionary for this component.
*/
public UserDataDictionary getUserData() {
Modified: pivot/trunk/wtk/src/org/apache/pivot/wtk/Window.java
URL: http://svn.apache.org/viewvc/pivot/trunk/wtk/src/org/apache/pivot/wtk/Window.java?rev=1712423&r1=1712422&r2=1712423&view=diff
==============================================================================
--- pivot/trunk/wtk/src/org/apache/pivot/wtk/Window.java (original)
+++ pivot/trunk/wtk/src/org/apache/pivot/wtk/Window.java Tue Nov 3
21:44:48 2015
@@ -25,6 +25,7 @@ import org.apache.pivot.collections.Hash
import org.apache.pivot.collections.Sequence;
import org.apache.pivot.util.ImmutableIterator;
import org.apache.pivot.util.ListenerList;
+import org.apache.pivot.util.Utils;
import org.apache.pivot.util.Vote;
import org.apache.pivot.wtk.media.Image;
@@ -76,9 +77,7 @@ public class Window extends Container {
if (keyStroke != previousKeyStroke) {
if (window != null) {
- if (keyStroke == null) {
- throw new IllegalStateException();
- }
+ Utils.checkNull(keyStroke, "keyStroke");
if (window.actionMap.containsKey(keyStroke)) {
throw new IllegalArgumentException("A mapping
for " + keyStroke
@@ -99,9 +98,7 @@ public class Window extends Container {
}
public void setKeyStroke(String keyStroke) {
- if (keyStroke == null) {
- throw new IllegalArgumentException("keyStroke is null.");
- }
+ Utils.checkNull(keyStroke, "keyStroke");
setKeyStroke(Keyboard.KeyStroke.decode(keyStroke));
}
@@ -115,9 +112,7 @@ public class Window extends Container {
if (action != previousAction) {
if (window != null) {
- if (action == null) {
- throw new IllegalStateException();
- }
+ Utils.checkNull(action, "action");
window.actionMap.put(keyStroke, action);
@@ -129,9 +124,7 @@ public class Window extends Container {
}
public void setAction(String actionID) {
- if (actionID == null) {
- throw new IllegalArgumentException("actionID is null");
- }
+ Utils.checkNull(actionID, "actionID");
Action actionLocal = Action.getNamedActions().get(actionID);
if (actionLocal == null) {
@@ -550,9 +543,7 @@ public class Window extends Container {
* window; <tt>false</tt>, otherwise.
*/
public boolean isOwner(Window window) {
- if (window == null) {
- throw new IllegalArgumentException("window is null.");
- }
+ Utils.checkNull(window, "window");
Window ownerLocal = window.getOwner();
@@ -599,9 +590,7 @@ public class Window extends Container {
* @throws IllegalArgumentException if the owner argument is {@code null}.
*/
public final void open(Window ownerArgument) {
- if (ownerArgument == null) {
- throw new IllegalArgumentException();
- }
+ Utils.checkNull(ownerArgument, "owner");
open(ownerArgument.getDisplay(), ownerArgument);
}
@@ -615,9 +604,7 @@ public class Window extends Container {
* has no owner.
*/
public void open(Display display, Window ownerArgument) {
- if (display == null) {
- throw new IllegalArgumentException("display is null.");
- }
+ Utils.checkNull(display, "display");
if (ownerArgument != null) {
if (!ownerArgument.isOpen()) {
@@ -784,9 +771,7 @@ public class Window extends Container {
* @param iconURL The location of the icon to set.
*/
public void setIcon(URL iconURL) {
- if (iconURL == null) {
- throw new IllegalArgumentException("iconURL is null.");
- }
+ Utils.checkNull(iconURL, "iconURL");
Image icon = Image.loadFromCache(iconURL);
@@ -802,14 +787,12 @@ public class Window extends Container {
* @see #setIcon(URL)
*/
public void setIcon(String iconName) {
- if (iconName == null) {
- throw new IllegalArgumentException("iconName is null.");
- }
+ Utils.checkNull(iconName, "iconName");
ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
URL url = classLoader.getResource(iconName.substring(1));
if (url == null) {
- throw new IllegalArgumentException("cannot find icon
resource " + iconName);
+ throw new IllegalArgumentException("Cannot find icon
resource " + iconName);
}
setIcon(url);
}