This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 89768

Summary: A class for checking method parameters
Product: platform Reporter: Andrei Badea <abadea>
Component: -- Other --Assignee: Andrei Badea <abadea>
Status: RESOLVED FIXED    
Severity: blocker CC: apireviews, pjiricka, vkraemer
Priority: P3 Keywords: API, API_REVIEW_FAST
Version: 6.x   
Hardware: All   
OS: All   
Issue Type: ENHANCEMENT Exception Reporter:
Attachments: Proposed change

Description Andrei Badea 2006-11-21 15:26:59 UTC
I just created the following class to ease checking method parameters:

public class Parameters {

    private Parameters() { }

    public static void notNull(CharSequence name, Object value) {
        if (value == null) {
            throw new NullPointerException("The " + name + " parameter cannot be
null"); // NOI18N
        }
    }

    public static void notEmpty(CharSequence name, CharSequence value) {
        notNull(name, value);
        if (value.length() == 0) {
            throw new IllegalArgumentException("The " + name + " parameter
cannot be an empty string"); // NOI18N
        }
    }

    public static void notWhitespace(CharSequence name, CharSequence value) {
        notNull(name, value);
        notEmpty(name, value.toString().trim());
    }

    public static void javaIdentifier(CharSequence name, CharSequence value) {
        notNull(name, value);
        javaIdentifierOrNull(name, value);
    }

    public static void javaIdentifierOrNull(CharSequence name, CharSequence value) {
        if (value != null && !Utilities.isJavaIdentifier(value.toString())) {
            throw new IllegalArgumentException("The " + name + " parameter is
not a valid Java identifier"); // NOI18N
        }
    }
}

It is already used in SourceUtils and GenerationUtils in j2ee/persistence, and 
seems to be more readable than if ...throw (not to mention that half of us seems
to throw NPE on null, while the other half prefers IAE). Would it make sense to
move it to openide/util? I will provide tests and Javadoc later if the reaction
is encouraging.
Comment 1 Jesse Glick 2006-11-21 17:05:05 UTC
Looks OK to me.


notWhitespace needs a different exc message.
Comment 2 Andrei Badea 2006-11-26 22:25:28 UTC
Created attachment 36286 [details]
Proposed change
Comment 3 Andrei Badea 2006-11-26 22:27:27 UTC
jglick: right, thanks for the catch.

Please review.
Comment 4 Jesse Glick 2006-11-27 18:49:18 UTC
s/java Identifier/Java identifier/
Comment 5 Andrei Badea 2006-12-06 14:27:49 UTC
jglick: thanks, will fix.

If there aren't any more comments I will integrate tomorrow. Thanks for the review.
Comment 6 Vince Kraemer 2006-12-06 15:54:09 UTC
Please stick to IAE.  The message in the IAE can tell the user it should not be
null, but the argument is illegal --> IllegalArgumentException makes more sense

For example: notWhitespace seems like it will throw both...

Why don't you have throws clauses on these methods?

i18n: all the messages will come from bundles before you integrate, right?
Comment 7 Jesse Glick 2006-12-06 16:13:38 UTC
Localized messages are inappropriate for argument checking at the API level. The
messages are intended for developers, not end users.
Comment 8 Andrei Badea 2006-12-06 17:24:55 UTC
Re. IAE vs. NPE: notWhitespace() indeed throws both. Throwing NPE for a null and
IAE for a not-null, but still illegal value, gives you more information. When
you pass a null the NPE tells you that the argument was null (and it was not
permitted). When you pass non-null the IAE implicitly tells you that the
argument was non-null, but it was not valid. There are many places in the JDK
where NPE is thrown on null, like the collections framework or the Javac API.

Re. throws clauses: not sure. Bloch recommends (item 44) not putting unchecked
exceptions in the method header. On the other hand some (even here in NetBeans)
do it. I don't have a strong opinion. Anyone?
Comment 9 Jesse Glick 2006-12-06 18:08:37 UTC
I think throws clauses for unchecked exceptions are unnecessary, so long as you
document them using @throws.
Comment 10 _ rkubacki 2006-12-06 18:35:32 UTC
I'd prefer not to add them to throws clause and just keep them documented. I
assume that typically we want to use these methods to verify some precondition
(similarly like assertions). In such case localization of error messages is not
needed too. You can still catch them if you need and do proper error handling
then including l10n.

Items 40 and 44 are really the most relevant in Bloch's book :-)
Comment 11 Andrei Badea 2006-12-08 14:38:50 UTC
Integrated.

Checking in apichanges.xml;
/cvs/openide/util/apichanges.xml,v  <--  apichanges.xml
new revision: 1.21; previous revision: 1.20
done
Checking in nbproject/project.properties;
/cvs/openide/util/nbproject/project.properties,v  <--  project.properties
new revision: 1.24; previous revision: 1.23
done
RCS file: /cvs/openide/util/src/org/openide/util/Parameters.java,v
done
Checking in src/org/openide/util/Parameters.java;
/cvs/openide/util/src/org/openide/util/Parameters.java,v  <--  Parameters.java
initial revision: 1.1
done
RCS file: /cvs/openide/util/test/unit/src/org/openide/util/ParametersTest.java,v
done
Checking in test/unit/src/org/openide/util/ParametersTest.java;
/cvs/openide/util/test/unit/src/org/openide/util/ParametersTest.java,v  <-- 
ParametersTest.java
initial revision: 1.1
done