commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jörg Schaible <joerg.schai...@gmx.de>
Subject Re: svn commit: r983219 - /commons/proper/lang/trunk/src/test/java/org/apache/commons/lang3/StringUtilsTest.java
Date Mon, 09 Aug 2010 07:15:02 GMT
sebb wrote:

> On 8 August 2010 22:34, James Carman <james@carmanconsulting.com> wrote:
>> To make sure I'm actually passing in the exact same values (by letting
>> the compiler create the arrays using the varargs feature) I added the
>> following output to the startsWithAny() method:
>>
>> System.out.println("startsWithAny(): Received " + string + " input
>> with " + (searchStrings == null ? null :
>> Arrays.asList(searchStrings).toString()) + " input array");
>>
>> When running the old version of the test code to the new version,
>> there was no difference in the output.
>>
>> I also ran a similar test by changing getCommonPrefix() method:
>>
>> System.out.println("getCommonPrefix(): Received " + (strs == null ?
>> null : Arrays.asList(strs).toString()) + " input array");
>>
>> Again, no changes in the output.  So, we're at least exercising the
>> "guts" of the methods the same.  Now I can understand that you might
>> be saying that we're not exercising how the code is called the same
>> way in both cases, but the varargs support vs. the manual array
>> creation is identical.  If you think we should add back in the manual
>> String[] stuff, I guess we could, but I really don't see much value in
>> it other than testing that varargs == manual String[].
>>
> 
> Although the compiler changes varargs into String[] arrays, the two
> tests are not equivalent.
> 
> Using varargs exclusively can hide bugs.
> 
> For example, method(String, String[]) is not the same as method(String
> ...).

While we're at it, looking at such a method, we should consider to test

method(string);

instead of its explicit equivalent

method(string, (String[])null);

Also, when introducing varargs we may not forget to update Javadoc. E.g. 
StringUitls.startsWithAny documents:

============= %< ================
     * StringUtils.startsWithAny(null, null)      = false
     * StringUtils.startsWithAny("abcxyz", null)     = false
============= %< ================

both calls are ambiguous now and will not compile. Actually there are now 4 
cases:

============= %< ================
     * StringUtils.startsWithAny(null)      = false
     * StringUtils.startsWithAny("abcxyz")     = false
     * StringUtils.startsWithAny(null, (String)null)      = false
     * StringUtils.startsWithAny("abcxyz", (String)null)     = false
============= %< ================

- Jörg


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message