commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r983137 - /commons/proper/lang/trunk/src/main/java/org/apache/commons/lang3/ArrayUtils.java
Date Mon, 09 Aug 2010 11:32:56 GMT
On 9 August 2010 04:45, James Carman <james@carmanconsulting.com> wrote:
> On Sun, Aug 8, 2010 at 10:04 PM, sebb <sebbaz@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


Mime
View raw message