On 4 March 2011 06:16, Henri Yandell <flame...@gmail.com> wrote:
> Going through each.
>> ArrayUtils.hashCode() has been removed, but it had different
>> functionality to Arrays.hashCode wrt nested arrays.

DONE

>> I don't love the new Pair class.

New thread.

>> ArrayUtils.toArray() javadoc has example code that won't compile
>> (missing "new") and also misses &lt; &gt; in places.

DONE

>> Some new methods use a different brace position from the existing
>> code, such as ArrayUtils.toArray(), Validate.matchesPattern(),
>> Validate.exclusiveBetween()...

DONE

>> Some new code isn't explicit about null handling, such as
>> AnnotationUtils, CharSequenceUtils, Validate.exclusiveBetween()...
> Do you mean that the javadoc doesn't explicitly say what happens when
> null is passed in?

Most of these have been done now. Arguments and results should say
"not null" or "may be null" or "null not recomended".

>> Some new methods don't have an @since, such as Validate.notBlank(),
>> Validate.exclusiveBetween()...

NOT done.

>> StringUtils is now an odd mixture of methods that accept a
>> CharSequence and ones that don't. Looking at it, I'd prefer to see
>> CharSequenceUtils added to, and StringUtils methods just deal with
>> Strings (A CharSequence might be mutable, so it has a different set of
>> implications when writing code using it). But if that isn't OK, then
>> it would be better to see everything in StringUtils take a
>> CharSequence.
>
> The aim is that everything in StringUtils that can reasonably take a
> CharSequence does; but to move a method to CharSequenceUtils simply
> because we made it use a higher interface is silly.
> Given the 1 method in CharSequenceUtils, discussing this makes me tend
> towards shoving it in the bloated StringUtils class.

If we're sticking with this as is then we should merge CharSeqUtils in.

More methods could take CharSequence too. and then convert to String
if necessary. Since this is the most important class, we need to get
it right.

>> ToStringStyle doesn't have a serialization version ID.

DONE

>> While we've moved away from NullArgumentException, I suspect it may be
>> reasonably widely used.
>
> People can make the case to add it back in :) Having Exceptions that
> weren't related to our actual API turned into an exercise in navel
> gazing.

Just saying that I think we may be asked where it went.

>> DateUtils has added a new MODIFY int enum, rather than using a real
>> enum. Nor has the existing RANGE int enum been converted

The Modify enum wasn't publicly used, so I made it private.

> I'm still very tempted to pull the entire package. It would be fun to
> port/submit the code into Joda Time and see what you parts you find
> acceptable there.

No, Date and Calendar here makes sense.

>> The JavaVersion name field is not used.

DONE (toString)

>> Range is only thread safe if the objects held inside are thread safe 
>> (javadoc).

DONE

>> Range.containsRange() might be better named containsAll()
>> Range.overlapsRange might be better named overlaps()

DONE

>> Public constants on StringEscapeUtils do not have javadoc.

LANG-682

>> The StringUtils.concat methods duplicate the existing join methods.

LANG-683.

More items:

The UTC constant in DateUtils isn't safe as TimeZone isn't immutable.
I copied the constant into DateFormatUtils for safety, but we should
probably remove it.

Spring AnnotatonUtils has a lot more useful methods. Expand in the future.

More <code>foo</code> to change to {@code foo}.

Stephen

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

Reply via email to