opennlp-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "william.colen@gmail.com" <william.co...@gmail.com>
Subject Re: svn commit: r1049639 - in /incubator/opennlp/trunk/opennlp-tools/src: main/java/opennlp/tools/formats/ main/java/opennlp/tools/util/ test/java/opennlp/tools/util/
Date Wed, 15 Dec 2010 18:09:56 GMT
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 <kottmann@gmail.com> 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
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message