You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by jc...@apache.org on 2010/08/07 00:44:39 UTC
svn commit: r983137 -
/commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
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")
+ 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]);
} else {
throw new IllegalArgumentException("Array element " + i + ", '"
+ object
Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
Posted by James Carman <ja...@carmanconsulting.com>.
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 <se...@gmail.com> wrote:
> On 10 August 2010 10:42, Julien Aymé <ju...@gmail.com> wrote:
>> 2010/8/10 sebb <se...@gmail.com>:
>>> On 9 August 2010 23:44, James Carman <ja...@carmanconsulting.com> wrote:
>>>> On Mon, Aug 9, 2010 at 7:32 AM, sebb <se...@gmail.com> 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: 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: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
Posted by sebb <se...@gmail.com>.
On 10 August 2010 10:42, Julien Aymé <ju...@gmail.com> wrote:
> 2010/8/10 sebb <se...@gmail.com>:
>> On 9 August 2010 23:44, James Carman <ja...@carmanconsulting.com> wrote:
>>> On Mon, Aug 9, 2010 at 7:32 AM, sebb <se...@gmail.com> 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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
Posted by Julien Aymé <ju...@gmail.com>.
2010/8/10 sebb <se...@gmail.com>:
> On 9 August 2010 23:44, James Carman <ja...@carmanconsulting.com> wrote:
>> On Mon, Aug 9, 2010 at 7:32 AM, sebb <se...@gmail.com> 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: 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
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
Posted by sebb <se...@gmail.com>.
On 9 August 2010 23:44, James Carman <ja...@carmanconsulting.com> wrote:
> On Mon, Aug 9, 2010 at 7:32 AM, sebb <se...@gmail.com> 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: 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: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
Posted by James Carman <ja...@carmanconsulting.com>.
On Mon, Aug 9, 2010 at 7:32 AM, sebb <se...@gmail.com> 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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
Posted by Jörg Schaible <jo...@gmx.de>.
Matt Benson wrote:
>
> On Aug 9, 2010, at 7:08 AM, James Carman wrote:
>
>> On Mon, Aug 9, 2010 at 7:32 AM, sebb <se...@gmail.com> 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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
Posted by Matt Benson <gu...@gmail.com>.
On Aug 9, 2010, at 7:08 AM, James Carman wrote:
> On Mon, Aug 9, 2010 at 7:32 AM, sebb <se...@gmail.com> 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: 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: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
Posted by Siegfried Goeschl <si...@it20one.at>.
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<se...@gmail.com> 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: 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: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
Posted by James Carman <ja...@carmanconsulting.com>.
On Mon, Aug 9, 2010 at 7:32 AM, sebb <se...@gmail.com> 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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
Posted by sebb <se...@gmail.com>.
On 9 August 2010 04:45, James Carman <ja...@carmanconsulting.com> wrote:
> On Sun, Aug 8, 2010 at 10:04 PM, sebb <se...@gmail.com> 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: 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: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
Posted by James Carman <ja...@carmanconsulting.com>.
On Sun, Aug 8, 2010 at 10:04 PM, sebb <se...@gmail.com> 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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
Posted by sebb <se...@gmail.com>.
On 8 August 2010 21:28, James Carman <ja...@carmanconsulting.com> wrote:
> On Sun, Aug 8, 2010 at 1:25 PM, sebb <se...@gmail.com> 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: 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: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
Posted by James Carman <ja...@carmanconsulting.com>.
On Sun, Aug 8, 2010 at 1:25 PM, sebb <se...@gmail.com> 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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
Posted by sebb <se...@gmail.com>.
On 8 August 2010 21:30, James Carman <ja...@carmanconsulting.com> wrote:
> On Sun, Aug 8, 2010 at 1:25 PM, sebb <se...@gmail.com> 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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
Posted by James Carman <ja...@carmanconsulting.com>.
On Sun, Aug 8, 2010 at 1:25 PM, sebb <se...@gmail.com> 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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
Posted by sebb <se...@gmail.com>.
On 6 August 2010 23:44, <jc...@apache.org> 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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org