You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Paul Benedict <pb...@apache.org> on 2009/11/16 19:16:26 UTC

Comments on Commons Lang 3.0-SNAPSHOT

* I think org.apache.commons.lang.math should be refactored and removed:
  * Fraction has nothing to do with this library and belongs in
Commons Math. I see it so closely align to higher Math functions and
not the JDK.
  * IEEE754rUtils should move to the root package
  * NumberUtils should move to the root package

* System is missing IS_JAVA_1_7 constant

* Validate throws IAE on every failure -- including null arguments.
Because NPE is not thrown, it will make this class less and less
palatable in the future since that is the favored approach nowadays --
and implemented by the JDK and Google. One solution is to expose a
pluggable factory that can choose the type of exception to throw.

* Is there any benefit to making NumberUtils.min/max accept a variable
arguments over a single array?

* I think the division of EscapeUtils and UnescapeUtils is
unnecessary. Do others think it's complex enough to make them separate
classes? I imagine so, but just checking.

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


Re: Comments on Commons Lang 3.0-SNAPSHOT

Posted by Paul Benedict <pb...@apache.org>.
Probably because both compile down into max(Object[]). But when I read
Joshua Bloch's Effective Java, he recommends doing something like
this:

max(double, double)
max(double, double, double)
max(double, double, double, double...)

That would be nice. The behind-the-sceneds Object[] creation only
happens if you inline 4. Is it worth the effort?

Paul

On Wed, Nov 18, 2009 at 12:37 AM, Henri Yandell <fl...@gmail.com> wrote:
> On Mon, Nov 16, 2009 at 11:48 PM, Henri Yandell <fl...@gmail.com> wrote:
>> On Mon, Nov 16, 2009 at 10:16 AM, Paul Benedict <pb...@apache.org> wrote:
>>> * Is there any benefit to making NumberUtils.min/max accept a variable
>>> arguments over a single array?
>>
>> Seems good.
>
> From LANG-396:
>
> NumberUtils.min, NumberUtils.max varargs runs into the no overloading
> recommendation:
>
> src/test/org/apache/commons/lang/math/NumberUtilsTest.java:[1175,42]
> reference to max is ambiguous, both method max(double...) in
> org.apache.commons.lang.math.NumberUtils and method max(float...) in
> org.apache.commons.lang.math.NumberUtils match
>
> So - tried that, had problems.
>
> Hen
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

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


Re: Comments on Commons Lang 3.0-SNAPSHOT

Posted by Henri Yandell <fl...@gmail.com>.
On Mon, Nov 16, 2009 at 11:48 PM, Henri Yandell <fl...@gmail.com> wrote:
> On Mon, Nov 16, 2009 at 10:16 AM, Paul Benedict <pb...@apache.org> wrote:
>> * Is there any benefit to making NumberUtils.min/max accept a variable
>> arguments over a single array?
>
> Seems good.

>From LANG-396:

NumberUtils.min, NumberUtils.max varargs runs into the no overloading
recommendation:

src/test/org/apache/commons/lang/math/NumberUtilsTest.java:[1175,42]
reference to max is ambiguous, both method max(double...) in
org.apache.commons.lang.math.NumberUtils and method max(float...) in
org.apache.commons.lang.math.NumberUtils match

So - tried that, had problems.

Hen

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


Re: Comments on Commons Lang 3.0-SNAPSHOT

Posted by Henri Yandell <fl...@gmail.com>.
On Mon, Nov 16, 2009 at 11:54 PM, Henri Yandell <fl...@gmail.com> wrote:
> On Mon, Nov 16, 2009 at 11:48 PM, Henri Yandell <fl...@gmail.com> wrote:

>> kill EscapeUtils/UnescapeUtils and make StringEscapeUtils undeprecated
>> and the intended front door.
>
> I'll do that. Delete EscapeUtils and UnescapeUtils; replace
> StringEscapeUtils with their merged contents.

Done. Snapshot javadoc has been updated.

Hen

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


Re: Comments on Commons Lang 3.0-SNAPSHOT

Posted by Henri Yandell <fl...@gmail.com>.
On Mon, Nov 16, 2009 at 11:48 PM, Henri Yandell <fl...@gmail.com> wrote:
> On Mon, Nov 16, 2009 at 10:16 AM, Paul Benedict <pb...@apache.org> wrote:
>
> Effectively, I think code on top of java.math is a fine fit for Lang,
> whereas Commons Math is a wholly different bigger beastie.
>
>> * System is missing IS_JAVA_1_7 constant
>
> Fair enough.

*cheers at the issue assigned to you*

>> * I think the division of EscapeUtils and UnescapeUtils is
>> unnecessary. Do others think it's complex enough to make them separate
>> classes? I imagine so, but just checking.
>
> It felt like a lot for one class, but we're the package with a near
> 6000 line StringUtils class, so 130 line classes are piddly. One thing
> I noticed is that I need to javadoc the Escape/UnescapeUtils classes.
> I'm happy to merge them if an EscapeUtils is preferred. Or even to
> kill EscapeUtils/UnescapeUtils and make StringEscapeUtils undeprecated
> and the intended front door.

I'll do that. Delete EscapeUtils and UnescapeUtils; replace
StringEscapeUtils with their merged contents.

Hen

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


Re: Comments on Commons Lang 3.0-SNAPSHOT

Posted by Paul Benedict <pb...@apache.org>.
>> * Validate throws IAE on every failure -- including null arguments.
>> Because NPE is not thrown, it will make this class less and less
>> palatable in the future since that is the favored approach nowadays --
>> and implemented by the JDK and Google. One solution is to expose a
>> pluggable factory that can choose the type of exception to throw.
>
> I count 137 throw IllegalArgumentException that say 'null' vs 45 throw
> NullPointerExceptions.  So this is a package wide decision to discuss.
> We used to have NullArgumentException having argued about it long ago,
> we recently deleted that and rolled back to IllegalArgumentException.
>

In regards to Validate, I think it's a BIG DEAL that it throws IAE not
NPE !!! I have deleted its use in several codebases because it does
not match what the "standard" is. And NullArgumentException rightfully
should die :-)

I understand the compatability concern, which is why I want to add the
ability to *plug* in the type of exception it creates. I'll work on
something so you guys can review it.

Paul

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


Re: Comments on Commons Lang 3.0-SNAPSHOT

Posted by Henri Yandell <fl...@gmail.com>.
On Mon, Nov 16, 2009 at 10:16 AM, Paul Benedict <pb...@apache.org> wrote:
> * I think org.apache.commons.lang.math should be refactored and removed:
>  * Fraction has nothing to do with this library and belongs in
> Commons Math. I see it so closely align to higher Math functions and
> not the JDK.

Dropping Fraction was discussed early on in 3.0, but consensus was for
the class to remain. It's simple enough and not been a maintenance
pain.

>  * IEEE754rUtils should move to the root package
>  * NumberUtils should move to the root package

Effectively, I think code on top of java.math is a fine fit for Lang,
whereas Commons Math is a wholly different bigger beastie.

> * System is missing IS_JAVA_1_7 constant

Fair enough.

> * Validate throws IAE on every failure -- including null arguments.
> Because NPE is not thrown, it will make this class less and less
> palatable in the future since that is the favored approach nowadays --
> and implemented by the JDK and Google. One solution is to expose a
> pluggable factory that can choose the type of exception to throw.

I count 137 throw IllegalArgumentException that say 'null' vs 45 throw
NullPointerExceptions.  So this is a package wide decision to discuss.
We used to have NullArgumentException having argued about it long ago,
we recently deleted that and rolled back to IllegalArgumentException.

> * Is there any benefit to making NumberUtils.min/max accept a variable
> arguments over a single array?

Seems good.

> * I think the division of EscapeUtils and UnescapeUtils is
> unnecessary. Do others think it's complex enough to make them separate
> classes? I imagine so, but just checking.

It felt like a lot for one class, but we're the package with a near
6000 line StringUtils class, so 130 line classes are piddly. One thing
I noticed is that I need to javadoc the Escape/UnescapeUtils classes.
I'm happy to merge them if an EscapeUtils is preferred. Or even to
kill EscapeUtils/UnescapeUtils and make StringEscapeUtils undeprecated
and the intended front door.

Hen

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