myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Simon Lessard <simon.lessar...@gmail.com>
Subject Re: [Trinidad] question about comment in Style.java code
Date Fri, 20 Mar 2009 15:46:11 GMT
Hi Jeanne,

Ok I checked the code back (and the comments) and here are the issues and
the meaning of the comment.

We have the following ArrayMap class that is used for different purpose,
most of the time as <String, String> or <String, Object>
public class ArrayMap<K, V> extends AbstractMap<K, V> implements Cloneable

The implementation's strategy is to store everything in a single array of
length = 2 * size() with the format:
array[keyIndex] = key;
array[keyIndex+1] = value;

Therefore, the container array HAS to be able to contain both K and V and
since those two classes have nothing in common but Object, then the
container array MUST be Object[]. Now all those type handling are hidden to
end users who use ArrayMap so there's no big deal there. However, BaseStyle
don't use the ArrayMap directly, but rather its various static method and
that's the real problem. So ArrayMap expose its internal behavior through
the following static methods used by BaseStyle:

public static Object get(Object[] array, Object key)
public static Iterator<Object> getKeys(Object[] array)
public static Object[] put(Object[] array, Object key, Object value)
public static Object[] remove(Object[] array, Object key)

The problem is with the getPropertyNames method that should return
Iterator<String>. However, it uses the getKeys(Object[]) which returns an
Iterator<Object> and it's not type-safe to cast that implicitly to an
Iterator<String> (although it would work with explicit and a
@SuppressWarnings("unchecked")). Or, we could enable generics on the getKeys
method:

public static <T> Iterator<T> getKeys(T[] array)

However now you iether have to use a String[] in BaseStyle, or cast the
Object[] to String[] in the method call. The latter works, the former is
more complicated. If you change the Object[] to String[] at class member
level then setProperty method will start failing because of the line:

_properties = ArrayMap.remove(_properties, name);

since the remove method returns an Object[] that is actually instanciated by
the ArrayMap class, casting it to String[] wouldn't be good to do at all
since it's really just a real Object[]. Now why not genericify remove as
well? Well let try

public static <T> T[] remove(T[] array, T key)

It seems good, but it's not as that method calls the genericified method

public static <T> T[] remove(T[] array, T key, boolean reallocate)

And since that method does some reallocation, everything crumble because new
T[] is never a valid line in Java because generics came so late and backward
compatibility is so saint that generics are just an illusion and are utterly
crappily integrated with Java, hence the dreaded type erasure. However,
arrays does not suffer that shortcoming so they're both covariant and
typesafe (ArrayStoreException) so both does NOT cohabit gracefully and you
cannot create a generic array at runtime because it has to be typesafe while
at runtime generics are not. Because of that, the remove method cannot be
generic enabled, so BaseStyle cannot use a String[]. That leaves you with
two options, use explicit cast or use an ArrayMap attribute rather than an
array and ArrayMap's static methods.


Do I make any sense?

~ Simon


On Thu, Mar 19, 2009 at 8:07 PM, Jeanne Waldman
<jeanne.waldman@oracle.com>wrote:

>
> Hi Simon,
>
> I have a question about your comment in the Style.java code.
>
>  /**
>  * Returns the names of the properties defined by this style.
>  */
>  // -= Simon Lessard =-
>  // FIXME: This should be changed to <String> once the issues
>  //        with ArrayMap are fixed. ATM (2006-08-04) ArrayMap
>  //        have huge problem working with anything but Object
>  public Iterator<Object> getPropertyNames();
>
>  /**
>  * Returns the value of the property with the specified name.
>  *
>  * @param name The property name for the property to return
>  */
>  public String getProperty(String name);
>
>
> What issues about ArrayMap are you talking about?
>
> I'm working on a task to make the Style object a public API,
> and I was going to change the api to:
>
> public Map<String, String> getProperties()
>
> I am concerned about your comment because I don't understand it.
> I see you have a comment in the ArrayMap code as well, and I see that Adam
> added a comment in
> response to your comment. If I use Map<String, String> getProperties and
> the implementation
> is using an ArrayMap do you think this won't work?
>
> Thanks,
> Jeanne
>
>
>

Mime
View raw message