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.
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.
Looks OK to me. notWhitespace needs a different exc message.
Created attachment 36286 [details] Proposed change
jglick: right, thanks for the catch. Please review.
s/java Identifier/Java identifier/
jglick: thanks, will fix. If there aren't any more comments I will integrate tomorrow. Thanks for the review.
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?
Localized messages are inappropriate for argument checking at the API level. The messages are intended for developers, not end users.
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?
I think throws clauses for unchecked exceptions are unnecessary, so long as you document them using @throws.
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 :-)
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