[JSF 2.0] Facelets package naming

[Simon Lessard <si...@gmail.com>]

Hi all,

I raised that issue before, but I came to change my mind about the naming
since the spec itself added extra packages including facelets. So, I'd like
to place MyFaces Facelets into package

org.apache.myfaces.webapp.pdl.facelets
or
org.apache.myfaces.pdl.facelets (since I don't think webapp makes any sense
there)

to reflect the JSF package javax.faces.webapp.pdl.facelets on the API side.

Also, in the long run (so not until we pass the TCK and everything more
important was already done), I think we should do the same with the JSP
classes, moving them all to

org.apache.myfaces.webapp.pdl.jsp
or
org.apache.myfaces.pdl.jsp


Which one do you prefer? Or do you have a better suggestion?


Regards,

~ Simon

Re: [JSF 2.0] Facelets package naming

[Leonardo Uribe <lu...@gmail.com>]

On Wed, Mar 18, 2009 at 11:01 AM, Matthias Wessendorf <ma...@apache.org>wrote:

> On Wed, Mar 18, 2009 at 3:27 PM, Simon Lessard
> <si...@gmail.com> wrote:
> > Hi all,
> >
> > I raised that issue before, but I came to change my mind about the naming
> > since the spec itself added extra packages including facelets. So, I'd
> like
> > to place MyFaces Facelets into package
> >
> > org.apache.myfaces.webapp.pdl.facelets
> > or
> > org.apache.myfaces.pdl.facelets (since I don't think webapp makes any
> sense
> > there)
>
> I agree on not much sense in webapp, however, I'd like to have the packed
> name:
> org.apache.myfaces.webapp.pdl.facelets
>
> >
> > to reflect the JSF package javax.faces.webapp.pdl.facelets on the API
> side.
> >
> > Also, in the long run (so not until we pass the TCK and everything more
> > important was already done), I think we should do the same with the JSP
> > classes, moving them all to
> >
> > org.apache.myfaces.webapp.pdl.jsp
> > or
> > org.apache.myfaces.pdl.jsp
> >
> >
> > Which one do you prefer? Or do you have a better suggestion?
>
> This is a pretty good idea. Again, I'd go for
> org.apache.myfaces.webapp.pdl.jsp
>
> -Matthias
>
> >
> >
> > Regards,
> >
> > ~ Simon
> >
>
>
>
> --
> Matthias Wessendorf
>
> blog: http://matthiaswessendorf.wordpress.com/
> sessions: http://www.slideshare.net/mwessendorf
> twitter: http://twitter.com/mwessendorf
>

Hi

 I'll go to 'webapp' variant, since it is intuitive associate it with
javax.faces.webapp.pdl, but really the word 'webapp' does not say anything.

regards

Leonardo Uribe

Re: [JSF 2.0] Facelets package naming

[Matthias Wessendorf <ma...@apache.org>]

On Wed, Mar 18, 2009 at 3:27 PM, Simon Lessard
<si...@gmail.com> wrote:
> Hi all,
>
> I raised that issue before, but I came to change my mind about the naming
> since the spec itself added extra packages including facelets. So, I'd like
> to place MyFaces Facelets into package
>
> org.apache.myfaces.webapp.pdl.facelets
> or
> org.apache.myfaces.pdl.facelets (since I don't think webapp makes any sense
> there)

I agree on not much sense in webapp, however, I'd like to have the packed name:
org.apache.myfaces.webapp.pdl.facelets

>
> to reflect the JSF package javax.faces.webapp.pdl.facelets on the API side.
>
> Also, in the long run (so not until we pass the TCK and everything more
> important was already done), I think we should do the same with the JSP
> classes, moving them all to
>
> org.apache.myfaces.webapp.pdl.jsp
> or
> org.apache.myfaces.pdl.jsp
>
>
> Which one do you prefer? Or do you have a better suggestion?

This is a pretty good idea. Again, I'd go for
org.apache.myfaces.webapp.pdl.jsp

-Matthias

>
>
> Regards,
>
> ~ Simon
>



-- 
Matthias Wessendorf

blog: http://matthiaswessendorf.wordpress.com/
sessions: http://www.slideshare.net/mwessendorf
twitter: http://twitter.com/mwessendorf

Re: [Trinidad] question about comment in Style.java code

[Simon Lessard <si...@gmail.com>]

Hi again,

Just before I get corrected by the gurus, I would like to add that it could
be possible to create a T[] through reflection magic, so that instead of

new T[], you could use

int[] dimension = {length - 2};
newArray = (T[])Array.newInstance(key.getClass(), dimension);

That would require a @SuppressWarnings("unchecked"), but it's also a viable
option although a bit slower. If you start enable generic on the static
methods of ArrayMap, please do it on all of them though.

p.s. That solution is viable, but has some potential shortcoming if used in
a different context like if the key's type was varying. For example, imagine
you have the following classes:
Shape
Circle extends Shape
Square extends Shape

Normally your classes would says 'hey, those are both shapes so I'll use
Shape[] as the storing array'. So you code everything and everything seems
to work when you call
_properties = remove(_properties, new Square()); // Now you think
'correctly' that T = Shape in the remove method

However, Array.newInstance don't use T, it uses getClass(), therefore the
returned value is of type Square[] then casted explicitely to Shape[] (which
is valid), but now your _properties array is really a Square[], no longer a
Shape[].

Time passes and you want to add a value with a Circle key, so you call

_properties = put(new Circle(), someValue); // Thinking T = Shape, which is
still correct

Sadly, when the put methods tried to insert the Circle in the array, the
system will try to put a Circle instance in a Square[] and therefore throw
an ArrayStoreException



Again hoping to make some sense,

~ Simon

On Fri, Mar 20, 2009 at 11:46 AM, Simon Lessard
<si...@gmail.com>wrote:

> 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
>>
>>
>>
>

Re: [Trinidad] question about comment in Style.java code

[Simon Lessard <si...@gmail.com>]

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
<je...@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
>
>
>

[Trinidad] question about comment in Style.java code

[Jeanne Waldman <je...@oracle.com>]

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