Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java

classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java

sebb-2-2
On 6 August 2010 23:44,  <[hidden email]> wrote:

> Author: jcarman
> Date: Fri Aug  6 22:44:38 2010
> New Revision: 983137
>
> URL: http://svn.apache.org/viewvc?rev=983137&view=rev
> Log:
> Generifying toMap() method (adding in possibility for type inference on return type).
>
> Modified:
>    commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
>
> Modified: commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
> URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java?rev=983137&r1=983136&r2=983137&view=diff
> ==============================================================================
> --- commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java (original)
> +++ commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java Fri Aug  6 22:44:38 2010
> @@ -16,6 +16,7 @@
>  */
>  package org.apache.commons.lang3;
>
> +import java.awt.Color;

???

>  import java.lang.reflect.Array;
>  import java.util.HashMap;
>  import java.util.Map;
> @@ -222,16 +223,17 @@ public class ArrayUtils {
>      * @throws IllegalArgumentException  if the array contains elements other
>      *  than {@link java.util.Map.Entry} and an Array
>      */
> -    public static Map<Object, Object> toMap(Object[] array) {
> +    @SuppressWarnings("unchecked")

Why is it safe to suppress warnings?

If it is really necessary, please document why it is safe, e.g. as is
done in ArrayUtils.addAll()


> +    public static <K,V> Map<K, V> toMap(Object[] array) {
>         if (array == null) {
>             return null;
>         }
> -        final Map<Object, Object> map = new HashMap<Object, Object>((int) (array.length * 1.5));
> +        final Map<K, V> map = new HashMap<K, V>((int) (array.length * 1.5));
>         for (int i = 0; i < array.length; i++) {
>             Object object = array[i];
>             if (object instanceof Map.Entry<?, ?>) {
>                 Map.Entry<?,?> entry = (Map.Entry<?,?>) object;
> -                map.put(entry.getKey(), entry.getValue());
> +                map.put((K)entry.getKey(), (V)entry.getValue());
>             } else if (object instanceof Object[]) {
>                 Object[] entry = (Object[]) object;
>                 if (entry.length < 2) {
> @@ -239,7 +241,7 @@ public class ArrayUtils {
>                         + object
>                         + "', has a length less than 2");
>                 }
> -                map.put(entry[0], entry[1]);
> +                map.put((K)entry[0], (V)entry[1]);

As it stands, the code allows calls of the form:

Map<Integer,Integer> map = ArrayUtils.toMap(new String[][] {{"foo",
"bar"}, {"hello", "world"}});

without complaining.

Failing to check the types of the array entries on creation means that
the error may lie undetected until much later, making it potentially
much harder to debug.

>             } else {
>                 throw new IllegalArgumentException("Array element " + i + ", '"
>                         + object
>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java

James Carman
On Sun, Aug 8, 2010 at 1:25 PM, sebb <[hidden email]> wrote:

>
> As it stands, the code allows calls of the form:
>
> Map<Integer,Integer> map = ArrayUtils.toMap(new String[][] {{"foo",
> "bar"}, {"hello", "world"}});
>
> without complaining.
>
> Failing to check the types of the array entries on creation means that
> the error may lie undetected until much later, making it potentially
> much harder to debug.
>

I am not against checking against the type arguments, but this change
assumes the user won't do something stupid.  It'd be a
ClassCastException later, yes.  Having the type arguments only allows
the user not to have to cast the return type.  They can use type
inference.

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java

James Carman
In reply to this post by sebb-2-2
On Sun, Aug 8, 2010 at 1:25 PM, sebb <[hidden email]> wrote:
>
> Why is it safe to suppress warnings?
>
> If it is really necessary, please document why it is safe, e.g. as is
> done in ArrayUtils.addAll()
>

It's safe because the code is making the assumption that the user
isn't trying to do something stupid.

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java

sebb-2-2
On 8 August 2010 21:30, James Carman <[hidden email]> wrote:

> On Sun, Aug 8, 2010 at 1:25 PM, sebb <[hidden email]> wrote:
>>
>> Why is it safe to suppress warnings?
>>
>> If it is really necessary, please document why it is safe, e.g. as is
>> done in ArrayUtils.addAll()
>>
>
> It's safe because the code is making the assumption that the user
> isn't trying to do something stupid.

In which case, this assumption needs to be documented as a // comment
on the annotation.

Also the type requirements aren't even mentioned in the Javadoc.

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java

sebb-2-2
In reply to this post by James Carman
On 8 August 2010 21:28, James Carman <[hidden email]> wrote:

> On Sun, Aug 8, 2010 at 1:25 PM, sebb <[hidden email]> wrote:
>>
>> As it stands, the code allows calls of the form:
>>
>> Map<Integer,Integer> map = ArrayUtils.toMap(new String[][] {{"foo",
>> "bar"}, {"hello", "world"}});
>>
>> without complaining.
>>
>> Failing to check the types of the array entries on creation means that
>> the error may lie undetected until much later, making it potentially
>> much harder to debug.
>>
>
> I am not against checking against the type arguments, but this change
> assumes the user won't do something stupid.

Or ignorant, as the type requirements are not documented.

> It'd be a ClassCastException later, yes.  Having the type arguments only allows
> the user not to have to cast the return type.  They can use type
> inference.

Yes.

But the current change makes it harder to find errors.

Previously, the compiler would warn about unsafe casts; with the
current implementation the warnings are suppressed.
IMO this is a retrograde step.

I expect generic library routines to create a Map that agrees with the
generic types, not allow the types to be violated.

Generics are partly about allowing the compiler to check that class
casts cannot happen.
This relies on generic methods behaving themselves, which the current
implementation does not.

> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java

James Carman
On Sun, Aug 8, 2010 at 10:04 PM, sebb <[hidden email]> wrote:

>
> Yes.
>
> But the current change makes it harder to find errors.
>
> Previously, the compiler would warn about unsafe casts; with the
> current implementation the warnings are suppressed.
> IMO this is a retrograde step.
>
> I expect generic library routines to create a Map that agrees with the
> generic types, not allow the types to be violated.
>
> Generics are partly about allowing the compiler to check that class
> casts cannot happen.
> This relies on generic methods behaving themselves, which the current
> implementation does not.
>

With type erasure, I believe it's impossible (correct me if I'm
wrong)to determine the exact type of "K" and "V" in the code so that
we could check the type safety of the keys/values passed in.  So, we
would either have to change the method signature to:

public Map<K,V> toMap(Class<K> keyType, Class<V> valueType, Object[] array);

or, we could add a warning to the documentation for the method:

"Warning: This code does not (and cannot) check the type safety of the
keys/values against the method's type variables (<K,V>).  Thus, if you
are expecting a Map<String,String> to be returned, but you pass in a
MapEntry<String,Integer>[] (or the analogous Object[][]), the method
will not fail, but you will get a ClassCastException at runtime."

Other than that, the only other option that I can see is to back out
the change completely, but then we lose the "syntactic sugar" that it
adds.  I personally like the sugar.  So, I'd say let's go with the
warning in the docs.

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java

sebb-2-2
On 9 August 2010 04:45, James Carman <[hidden email]> wrote:

> On Sun, Aug 8, 2010 at 10:04 PM, sebb <[hidden email]> wrote:
>>
>> Yes.
>>
>> But the current change makes it harder to find errors.
>>
>> Previously, the compiler would warn about unsafe casts; with the
>> current implementation the warnings are suppressed.
>> IMO this is a retrograde step.
>>
>> I expect generic library routines to create a Map that agrees with the
>> generic types, not allow the types to be violated.
>>
>> Generics are partly about allowing the compiler to check that class
>> casts cannot happen.
>> This relies on generic methods behaving themselves, which the current
>> implementation does not.
>>
>
> With type erasure, I believe it's impossible (correct me if I'm
> wrong)to determine the exact type of "K" and "V" in the code so that
> we could check the type safety of the keys/values passed in.  So, we
> would either have to change the method signature to:
>
> public Map<K,V> toMap(Class<K> keyType, Class<V> valueType, Object[] array);
>
> or, we could add a warning to the documentation for the method:
>
> "Warning: This code does not (and cannot) check the type safety of the
> keys/values against the method's type variables (<K,V>).  Thus, if you
> are expecting a Map<String,String> to be returned, but you pass in a
> MapEntry<String,Integer>[] (or the analogous Object[][]), the method
> will not fail, but you will get a ClassCastException at runtime."
>
> Other than that, the only other option that I can see is to back out
> the change completely, but then we lose the "syntactic sugar" that it
> adds.  I personally like the sugar.

The problem is that this sugar rots the teeth of the generic
type-checking system.

> So, I'd say let's go with the warning in the docs.

Why not split the code into two methods:

public static <K,V> Map<K, V> toMap(Map.Entry<K,V>[] array)
and
public static <K,V> Map<K, V> toMap(Class<K> keyType, Class<V>
valueType, Object[][] array)

Both methods can be made type-safe.

Failing that, the current method could at least check that all pairs
are of the same types, and AFAICT one can still add the first method
as an overloaded version which is type-safe.

I think the Object[][] version should also check that the array
entries are exactly length 2 - it's nonsense to pass in a longer
array.

> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java

James Carman
On Mon, Aug 9, 2010 at 7:32 AM, sebb <[hidden email]> wrote:
> Why not split the code into two methods:
>
> public static <K,V> Map<K, V> toMap(Map.Entry<K,V>[] array)
> and
> public static <K,V> Map<K, V> toMap(Class<K> keyType, Class<V>
> valueType, Object[][] array)
>
> Both methods can be made type-safe.
>

Well, we did just bump the version number, so we are free to do
something like that I guess.  Anyone else have any thoughts on the
subject (are you even paying attention anymore)?

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java

Siegfried Goeschl
Hi James,

we are paying attention but it is hard to provide useful comments if you
are distracted with payed work ... :-)

As far as I understand the discussion I would lean towards Sebastion
reasoning - but still wondering how he finds the time to review every
single commit ... :-)

Cheers,

Siegfried Goeschl

On 09.08.10 14:08, James Carman wrote:

> On Mon, Aug 9, 2010 at 7:32 AM, sebb<[hidden email]>  wrote:
>> Why not split the code into two methods:
>>
>> public static<K,V>  Map<K, V>  toMap(Map.Entry<K,V>[] array)
>> and
>> public static<K,V>  Map<K, V>  toMap(Class<K>  keyType, Class<V>
>> valueType, Object[][] array)
>>
>> Both methods can be made type-safe.
>>
>
> Well, we did just bump the version number, so we are free to do
> something like that I guess.  Anyone else have any thoughts on the
> subject (are you even paying attention anymore)?
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java

Matt Benson-2
In reply to this post by James Carman

On Aug 9, 2010, at 7:08 AM, James Carman wrote:

> On Mon, Aug 9, 2010 at 7:32 AM, sebb <[hidden email]> wrote:
>> Why not split the code into two methods:
>>
>> public static <K,V> Map<K, V> toMap(Map.Entry<K,V>[] array)
>> and
>> public static <K,V> Map<K, V> toMap(Class<K> keyType, Class<V>
>> valueType, Object[][] array)
>>
>> Both methods can be made type-safe.
>>
>
> Well, we did just bump the version number, so we are free to do
> something like that I guess.  Anyone else have any thoughts on the
> subject (are you even paying attention anymore)?
>

In the spirit of the Map.Entry variant of the method, I would suggest eating our own lang3 dog food and providing:

public static <K, V> Map<K, V> toMap(Pair<K, V>... entries)

as well.  Pair.of("foo", "bar") is certainly a terser option than creating an anonymous implementation of Map.Entry.

-Matt

> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java

Jörg Schaible
Matt Benson wrote:

>
> On Aug 9, 2010, at 7:08 AM, James Carman wrote:
>
>> On Mon, Aug 9, 2010 at 7:32 AM, sebb <[hidden email]> wrote:
>>> Why not split the code into two methods:
>>>
>>> public static <K,V> Map<K, V> toMap(Map.Entry<K,V>[] array)
>>> and
>>> public static <K,V> Map<K, V> toMap(Class<K> keyType, Class<V>
>>> valueType, Object[][] array)
>>>
>>> Both methods can be made type-safe.
>>>
>>
>> Well, we did just bump the version number, so we are free to do
>> something like that I guess.  Anyone else have any thoughts on the
>> subject (are you even paying attention anymore)?
>>
>
> In the spirit of the Map.Entry variant of the method, I would suggest
> eating our own lang3 dog food and providing:
>
> public static <K, V> Map<K, V> toMap(Pair<K, V>... entries)
>
> as well.  Pair.of("foo", "bar") is certainly a terser option than creating
> an anonymous implementation of Map.Entry.

+1

I also thought about splitting the API, but lack of an idea I was quiet ;-)

- Jörg


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java

James Carman
In reply to this post by sebb-2-2
On Mon, Aug 9, 2010 at 7:32 AM, sebb <[hidden email]> wrote:
> Why not split the code into two methods:
>
> public static <K,V> Map<K, V> toMap(Map.Entry<K,V>[] array)
> and
> public static <K,V> Map<K, V> toMap(Class<K> keyType, Class<V>
> valueType, Object[][] array)
>

So, what would that make the "user" code look like?  The whole reason
for this, unless I'm mistaken, was to make it easy to do stuff like:

public static final Map<String,Color> COLOR_MAP = ArrayUtils.toMap(new
Object[][] {
  { "red", Color.red },
  { "blue", Color.blue }
} );

To me, we're really trying to make it easier to use this API, right?
So, now the code would look like:

public static final Map<String,Color> COLOR_MAP =
ArrayUtils.toMap(String.class, Color.class, new Object[][] {
  { "red", Color.red },
  { "blue", Color.blue }
} );

This is one of the big gripes about how generics are handled in Java,
you have to repeat yourself so darn much.  What if we told folks to
use a new MapBuilder instead?

public static final Map<String,Color> COLOR_MAP =
MapUtils.builder().put("red", Color.red).put("blue",
Color.blue).toMap();

It seems like this "builder" stuff is getting popular these days.
What do you guys think?

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java

sebb-2-2
On 9 August 2010 23:44, James Carman <[hidden email]> wrote:

> On Mon, Aug 9, 2010 at 7:32 AM, sebb <[hidden email]> wrote:
>> Why not split the code into two methods:
>>
>> public static <K,V> Map<K, V> toMap(Map.Entry<K,V>[] array)
>> and
>> public static <K,V> Map<K, V> toMap(Class<K> keyType, Class<V>
>> valueType, Object[][] array)
>>
>
> So, what would that make the "user" code look like?  The whole reason
> for this, unless I'm mistaken, was to make it easy to do stuff like:
>
> public static final Map<String,Color> COLOR_MAP = ArrayUtils.toMap(new
> Object[][] {
>  { "red", Color.red },
>  { "blue", Color.blue }
> } );
>
> To me, we're really trying to make it easier to use this API, right?
> So, now the code would look like:
>
> public static final Map<String,Color> COLOR_MAP =
> ArrayUtils.toMap(String.class, Color.class, new Object[][] {
>  { "red", Color.red },
>  { "blue", Color.blue }
> } );
>

Looks OK to me, and solves the type-safety problem.

> This is one of the big gripes about how generics are handled in Java,
> you have to repeat yourself so darn much.  What if we told folks to
> use a new MapBuilder instead?
>
> public static final Map<String,Color> COLOR_MAP =
> MapUtils.builder().put("red", Color.red).put("blue",
> Color.blue).toMap();
>
> It seems like this "builder" stuff is getting popular these days.
> What do you guys think?

AFAICT, that does not ensure type-safety, so is no better than the current API.

> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java

Julien Aymé
2010/8/10 sebb <[hidden email]>:

> On 9 August 2010 23:44, James Carman <[hidden email]> wrote:
>> On Mon, Aug 9, 2010 at 7:32 AM, sebb <[hidden email]> wrote:
>>> Why not split the code into two methods:
>>>
>>> public static <K,V> Map<K, V> toMap(Map.Entry<K,V>[] array)
>>> and
>>> public static <K,V> Map<K, V> toMap(Class<K> keyType, Class<V>
>>> valueType, Object[][] array)
>>>
>>
>> So, what would that make the "user" code look like?  The whole reason
>> for this, unless I'm mistaken, was to make it easy to do stuff like:
>>
>> public static final Map<String,Color> COLOR_MAP = ArrayUtils.toMap(new
>> Object[][] {
>>  { "red", Color.red },
>>  { "blue", Color.blue }
>> } );
>>
>> To me, we're really trying to make it easier to use this API, right?
>> So, now the code would look like:
>>
>> public static final Map<String,Color> COLOR_MAP =
>> ArrayUtils.toMap(String.class, Color.class, new Object[][] {
>>  { "red", Color.red },
>>  { "blue", Color.blue }
>> } );
>>
>
> Looks OK to me, and solves the type-safety problem.
>
>> This is one of the big gripes about how generics are handled in Java,
>> you have to repeat yourself so darn much.  What if we told folks to
>> use a new MapBuilder instead?
>>
>> public static final Map<String,Color> COLOR_MAP =
>> MapUtils.builder().put("red", Color.red).put("blue",
>> Color.blue).toMap();
>>
>> It seems like this "builder" stuff is getting popular these days.
>> What do you guys think?
>
> AFAICT, that does not ensure type-safety, so is no better than the current API.
>

The builder can ensure type-safety, if the builder itself is generic:
MapBuilder<K, V>, with methods:
- void put(K key, V value)
- Map<K, V> toMap()

and MapUtils method is:
public static <K, V> MapBuilder<K, V> builder();

(And the MapBuilder looks awfully a lot like Map ;-) )

>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java

sebb-2-2
On 10 August 2010 10:42, Julien Aymé <[hidden email]> wrote:

> 2010/8/10 sebb <[hidden email]>:
>> On 9 August 2010 23:44, James Carman <[hidden email]> wrote:
>>> On Mon, Aug 9, 2010 at 7:32 AM, sebb <[hidden email]> wrote:
>>>> Why not split the code into two methods:
>>>>
>>>> public static <K,V> Map<K, V> toMap(Map.Entry<K,V>[] array)
>>>> and
>>>> public static <K,V> Map<K, V> toMap(Class<K> keyType, Class<V>
>>>> valueType, Object[][] array)
>>>>
>>>
>>> So, what would that make the "user" code look like?  The whole reason
>>> for this, unless I'm mistaken, was to make it easy to do stuff like:
>>>
>>> public static final Map<String,Color> COLOR_MAP = ArrayUtils.toMap(new
>>> Object[][] {
>>>  { "red", Color.red },
>>>  { "blue", Color.blue }
>>> } );
>>>
>>> To me, we're really trying to make it easier to use this API, right?
>>> So, now the code would look like:
>>>
>>> public static final Map<String,Color> COLOR_MAP =
>>> ArrayUtils.toMap(String.class, Color.class, new Object[][] {
>>>  { "red", Color.red },
>>>  { "blue", Color.blue }
>>> } );
>>>
>>
>> Looks OK to me, and solves the type-safety problem.
>>
>>> This is one of the big gripes about how generics are handled in Java,
>>> you have to repeat yourself so darn much.  What if we told folks to
>>> use a new MapBuilder instead?
>>>
>>> public static final Map<String,Color> COLOR_MAP =
>>> MapUtils.builder().put("red", Color.red).put("blue",
>>> Color.blue).toMap();
>>>
>>> It seems like this "builder" stuff is getting popular these days.
>>> What do you guys think?
>>
>> AFAICT, that does not ensure type-safety, so is no better than the current API.
>>
>
> The builder can ensure type-safety, if the builder itself is generic:

Of course, but the user code will be more verbose.

> MapBuilder<K, V>, with methods:
> - void put(K key, V value)
> - Map<K, V> toMap()
>
> and MapUtils method is:
> public static <K, V> MapBuilder<K, V> builder();
>
> (And the MapBuilder looks awfully a lot like Map ;-) )

But what would the code look like?

Would it be any neater than:

public static final Map<String,Color> COLOR_MAP =
ArrayUtils.toMap(String.class, Color.class, new Object[][] {
 { "red", Color.red },
 { "blue", Color.blue }
} );

I suspect not.

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java

James Carman
FYI, I have reverted the offensive code.  I do not have the time (nor
the desire) to discuss every little commit I make ad-nauseum.

ConstructorUtils fails to compile in trunk right now, btw.

On Tue, Aug 10, 2010 at 5:52 AM, sebb <[hidden email]> wrote:

> On 10 August 2010 10:42, Julien Aymé <[hidden email]> wrote:
>> 2010/8/10 sebb <[hidden email]>:
>>> On 9 August 2010 23:44, James Carman <[hidden email]> wrote:
>>>> On Mon, Aug 9, 2010 at 7:32 AM, sebb <[hidden email]> wrote:
>>>>> Why not split the code into two methods:
>>>>>
>>>>> public static <K,V> Map<K, V> toMap(Map.Entry<K,V>[] array)
>>>>> and
>>>>> public static <K,V> Map<K, V> toMap(Class<K> keyType, Class<V>
>>>>> valueType, Object[][] array)
>>>>>
>>>>
>>>> So, what would that make the "user" code look like?  The whole reason
>>>> for this, unless I'm mistaken, was to make it easy to do stuff like:
>>>>
>>>> public static final Map<String,Color> COLOR_MAP = ArrayUtils.toMap(new
>>>> Object[][] {
>>>>  { "red", Color.red },
>>>>  { "blue", Color.blue }
>>>> } );
>>>>
>>>> To me, we're really trying to make it easier to use this API, right?
>>>> So, now the code would look like:
>>>>
>>>> public static final Map<String,Color> COLOR_MAP =
>>>> ArrayUtils.toMap(String.class, Color.class, new Object[][] {
>>>>  { "red", Color.red },
>>>>  { "blue", Color.blue }
>>>> } );
>>>>
>>>
>>> Looks OK to me, and solves the type-safety problem.
>>>
>>>> This is one of the big gripes about how generics are handled in Java,
>>>> you have to repeat yourself so darn much.  What if we told folks to
>>>> use a new MapBuilder instead?
>>>>
>>>> public static final Map<String,Color> COLOR_MAP =
>>>> MapUtils.builder().put("red", Color.red).put("blue",
>>>> Color.blue).toMap();
>>>>
>>>> It seems like this "builder" stuff is getting popular these days.
>>>> What do you guys think?
>>>
>>> AFAICT, that does not ensure type-safety, so is no better than the current API.
>>>
>>
>> The builder can ensure type-safety, if the builder itself is generic:
>
> Of course, but the user code will be more verbose.
>
>> MapBuilder<K, V>, with methods:
>> - void put(K key, V value)
>> - Map<K, V> toMap()
>>
>> and MapUtils method is:
>> public static <K, V> MapBuilder<K, V> builder();
>>
>> (And the MapBuilder looks awfully a lot like Map ;-) )
>
> But what would the code look like?
>
> Would it be any neater than:
>
> public static final Map<String,Color> COLOR_MAP =
> ArrayUtils.toMap(String.class, Color.class, new Object[][] {
>  { "red", Color.red },
>  { "blue", Color.blue }
> } );
>
> I suspect not.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]