Hi Jörn, I couldn't understand what you mean with "I think its nice to just do it in a util method instead." Should I move the method StringUtil.isEmpty to another class?
I'll throw the NPE if the string is null. Thanks, William On Wed, Dec 15, 2010 at 3:52 PM, Jörn Kottmann <[email protected]> wrote: > Hi William, > > reviewed your commit, comments are embedded below: > >> Modified: >> incubator/opennlp/trunk/opennlp-tools/src/main/java/opennlp/tools/util/StringUtil.java >> URL: >> http://svn.apache.org/viewvc/incubator/opennlp/trunk/opennlp-tools/src/main/java/opennlp/tools/util/StringUtil.java?rev=1049639&r1=1049638&r2=1049639&view=diff >> >> ============================================================================== >> --- >> incubator/opennlp/trunk/opennlp-tools/src/main/java/opennlp/tools/util/StringUtil.java >> (original) >> +++ >> incubator/opennlp/trunk/opennlp-tools/src/main/java/opennlp/tools/util/StringUtil.java >> Wed Dec 15 16:44:06 2010 >> @@ -100,4 +100,17 @@ public class StringUtil { >> >> return new String(upperCaseChars); >> } >> + >> + /** >> + * Returns<tt>true</tt> if {...@link CharSequence#length()} is >> + *<tt>0</tt> or<tt>null</tt>. >> + * >> + * @return<tt>true</tt> if {...@link CharSequence#length()} >> is<tt>0</tt>, otherwise >> + *<tt>false</tt> >> + * >> + * @since 1.5.1 >> + */ >> + public static boolean isEmpty(CharSequence theString) { >> + return theString == null || theString.length() == 0; >> + } >> } >> > > I think its nice to just do it in a util method instead. One day we > switch to java 6 (or greater) we can simply mark it as deprecated > and use String.isEmpty() again. When we do this we cannot > tolerate null strings, because that will just throw a NullPointerException. > Thats why I think we should no tolerate a null value, and throw a NPE when > we get > one here. > > Modified: >> incubator/opennlp/trunk/opennlp-tools/src/test/java/opennlp/tools/util/StringUtilTest.java >> URL: >> http://svn.apache.org/viewvc/incubator/opennlp/trunk/opennlp-tools/src/test/java/opennlp/tools/util/StringUtilTest.java?rev=1049639&r1=1049638&r2=1049639&view=diff >> >> ============================================================================== >> --- >> incubator/opennlp/trunk/opennlp-tools/src/test/java/opennlp/tools/util/StringUtilTest.java >> (original) >> +++ >> incubator/opennlp/trunk/opennlp-tools/src/test/java/opennlp/tools/util/StringUtilTest.java >> Wed Dec 15 16:44:06 2010 >> @@ -49,5 +49,12 @@ public class StringUtilTest { >> assertEquals("TEST", StringUtil.toUpperCase("test")); >> assertEquals("SIMPLE", StringUtil.toUpperCase("simple")); >> } >> + >> + @Test >> + public void testIsEmpty() { >> + assertTrue(StringUtil.isEmpty(null)); >> + assertTrue(StringUtil.isEmpty("")); >> + assertTrue(!StringUtil.isEmpty("a")); >> + } >> >> } >> >> Thanks for the test :) > > Jörn >
