You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by Andrew Robinson <an...@gmail.com> on 2008/01/23 19:46:19 UTC

[Trinidad] CoreRenderer static methods

There are static methods on CoreRenderer that I have no idea why they are
static. The two in particular that I noticed:

static public void renderStyleClass
static public void renderStyleClasses

These are methods for renderers, why would they be static?

There are some util style classes that are calling them, because of this, I
would think these methods should be moved into a Util class and have the
Renderer methods invoke the Util ones. IMO, static utility functions do not
belong on classes that are meant to be extended. Do others agree, or am I
solo on this opinion?

-Andrew

Re: [Trinidad] CoreRenderer static methods

Posted by Matthias Wessendorf <ma...@apache.org>.
>  In the future, I think we should make that method as well as PropertyKey
> class generic aware.
>  I completely agree.  I did this on a branch one night but never got around
> to submitting the patch.

that would be great.

-Matthias

>
>  -- Blake Sullivan
>
>
>
>
>
>
>  Regards,
>
>  ~ Simon
>
>    /**
>     * This method evaluates the property of the specified bean if it
> supports
>     * the specified property key and if the current value is
> <code>null</code>,
>     * then evaluate the default value.
>     * <p>
>     * If the bean does not support the specified key, this method returns
>     * <code>null</code>. Unsupported keys occur when the bean's type its
> type
>     * returned <code>null</code> from <code>findKey</code> when
>     * <code>findTypeConstants</code> method was called.
>     * </p>
>     *
>     * @param bean the property value holder.
>     * @param key  the key associated to the property to evaluate.
>     *
>     * @return <code>null</code> if key is <code>null</code>, the current
>     *         property value in the bean for the specified key if it was
>     *         set, or the default value if it wasn't.
>     *
>     * @see
> #findTypeConstants(org.apache.myfaces.trinidad.bean.FacesBean.Type)
>     */
>    protected Object resolveProperty(FacesBean bean, PropertyKey key)
>    {
>      if (key == null)
>      {
>        return null;
>      }
>
>      Object value = bean.getProperty(key);
>      if (value == null)
>      {
>        value = key.getDefault();
>      }
>
>      return value;
>    }
>
>
>
>
> On Jan 23, 2008 7:13 PM, Blake Sullivan <bl...@oracle.com> wrote:
>
> >
> >
> > Andrew Robinson wrote:
> > > There are static methods on CoreRenderer that I have no idea why they
> > > are static. The two in particular that I noticed:
> > >
> > > static public void renderStyleClass
> > > static public void renderStyleClasses
> > >
> > > These are methods for renderers, why would they be static?
> > >
> > > There are some util style classes that are calling them, because of
> > > this, I would think these methods should be moved into a Util class
> > > and have the Renderer methods invoke the Util ones. IMO, static
> > > utility functions do not belong on classes that are meant to be
> > > extended. Do others agree, or am I solo on this opinion?
> > Andrew, isn't the real problem here that these methods should be on
> > XhtmlRenderer or XhtmlUtils as their implementations are
> > Xhtml-specific.  I don't have a problem per se with static methods on
> > extendable classes.  However, if the class is going to contain
> > extractable static convenience functions, then it is confusing to have
> > both the class itself and a separate Utils class.  Given that we have a
> > separate Utils class, these methods shouldn't be on the abstract class.
> >
> > -- Blake Sullivan
> > >
> > > -Andrew
> >
> >
>
>
>



-- 
Matthias Wessendorf

further stuff:
blog: http://matthiaswessendorf.wordpress.com/
sessions: http://www.slideshare.net/mwessendorf
mail: matzew-at-apache-dot-org

Re: [Trinidad] CoreRenderer static methods

Posted by Blake Sullivan <bl...@oracle.com>.
Simon Lessard wrote:
> I agree that they should be on XhtmlRenderer since it's XHTML 
> specific, but I also agree with Andrew on the static part. There are 
> many methods in Trinidad that are static for a reason that's oblivious 
> to me and those two are part of them.
I didn't think there was any argument over these methods being static.  
The only reason for making these instance methods would be if we were 
claiming that we expect that the Renderer needs to change the behavior 
in some cases.
>
> While on the XhtmlRenderer vs CoreRenderer issue, I think we should 
> more XhtmlRenderer to API even if it's XHTML specific since it's so 
> useful and since HTML is the most common rendered markup for Trinidad. 
> Also, I'd like to graduate the findTypeConstants and resolveProperty 
> methods from XhtmlRenderer to CoreRenderer except that I would remove 
> the boolean flag from the latter since it's quite useless.
>
> In the future, I think we should make that method as well as 
> PropertyKey class generic aware.
I completely agree.  I did this on a branch one night but never got 
around to submitting the patch.

-- Blake Sullivan

>
>
> Regards,
>
> ~ Simon
>
>   /**
>    * This method evaluates the property of the specified bean if it 
> supports
>    * the specified property key and if the current value is 
> <code>null</code>,
>    * then evaluate the default value.
>    * <p>
>    * If the bean does not support the specified key, this method returns
>    * <code>null</code>. Unsupported keys occur when the bean's type 
> its type
>    * returned <code>null</code> from <code>findKey</code> when
>    * <code>findTypeConstants</code> method was called.
>    * </p>
>    *
>    * @param bean the property value holder.
>    * @param key  the key associated to the property to evaluate.
>    *
>    * @return <code>null</code> if key is <code>null</code>, the current
>    *         property value in the bean for the specified key if it was
>    *         set, or the default value if it wasn't.
>    *
>    * @see 
> #findTypeConstants(org.apache.myfaces.trinidad.bean.FacesBean.Type)
>    */
>   protected Object resolveProperty(FacesBean bean, PropertyKey key)
>   {
>     if (key == null)
>     {
>       return null;
>     }
>    
>     Object value = bean.getProperty(key);
>     if (value == null)
>     {
>       value = key.getDefault();
>     }
>    
>     return value;
>   }
>  
>
>
> On Jan 23, 2008 7:13 PM, Blake Sullivan <blake.sullivan@oracle.com 
> <ma...@oracle.com>> wrote:
>
>     Andrew Robinson wrote:
>     > There are static methods on CoreRenderer that I have no idea why
>     they
>     > are static. The two in particular that I noticed:
>     >
>     > static public void renderStyleClass
>     > static public void renderStyleClasses
>     >
>     > These are methods for renderers, why would they be static?
>     >
>     > There are some util style classes that are calling them, because of
>     > this, I would think these methods should be moved into a Util class
>     > and have the Renderer methods invoke the Util ones. IMO, static
>     > utility functions do not belong on classes that are meant to be
>     > extended. Do others agree, or am I solo on this opinion?
>     Andrew, isn't the real problem here that these methods should be on
>     XhtmlRenderer or XhtmlUtils as their implementations are
>     Xhtml-specific.  I don't have a problem per se with static methods on
>     extendable classes.  However, if the class is going to contain
>     extractable static convenience functions, then it is confusing to have
>     both the class itself and a separate Utils class.  Given that we
>     have a
>     separate Utils class, these methods shouldn't be on the abstract
>     class.
>
>     -- Blake Sullivan
>     >
>     > -Andrew
>
>


Re: [Trinidad] CoreRenderer static methods

Posted by Simon Lessard <si...@gmail.com>.
I agree that they should be on XhtmlRenderer since it's XHTML specific, but
I also agree with Andrew on the static part. There are many methods in
Trinidad that are static for a reason that's oblivious to me and those two
are part of them.

While on the XhtmlRenderer vs CoreRenderer issue, I think we should more
XhtmlRenderer to API even if it's XHTML specific since it's so useful and
since HTML is the most common rendered markup for Trinidad. Also, I'd like
to graduate the findTypeConstants and resolveProperty methods from
XhtmlRenderer to CoreRenderer except that I would remove the boolean flag
from the latter since it's quite useless.

In the future, I think we should make that method as well as PropertyKey
class generic aware.


Regards,

~ Simon

  /**
   * This method evaluates the property of the specified bean if it supports

   * the specified property key and if the current value is
<code>null</code>,
   * then evaluate the default value.
   * <p>
   * If the bean does not support the specified key, this method returns
   * <code>null</code>. Unsupported keys occur when the bean's type its type

   * returned <code>null</code> from <code>findKey</code> when
   * <code>findTypeConstants</code> method was called.
   * </p>
   *
   * @param bean the property value holder.
   * @param key  the key associated to the property to evaluate.
   *
   * @return <code>null</code> if key is <code>null</code>, the current
   *         property value in the bean for the specified key if it was
   *         set, or the default value if it wasn't.
   *
   * @see #findTypeConstants(org.apache.myfaces.trinidad.bean.FacesBean.Type
)
   */
  protected Object resolveProperty(FacesBean bean, PropertyKey key)
  {
    if (key == null)
    {
      return null;
    }

    Object value = bean.getProperty(key);
    if (value == null)
    {
      value = key.getDefault();
    }

    return value;
  }



On Jan 23, 2008 7:13 PM, Blake Sullivan <bl...@oracle.com> wrote:

> Andrew Robinson wrote:
> > There are static methods on CoreRenderer that I have no idea why they
> > are static. The two in particular that I noticed:
> >
> > static public void renderStyleClass
> > static public void renderStyleClasses
> >
> > These are methods for renderers, why would they be static?
> >
> > There are some util style classes that are calling them, because of
> > this, I would think these methods should be moved into a Util class
> > and have the Renderer methods invoke the Util ones. IMO, static
> > utility functions do not belong on classes that are meant to be
> > extended. Do others agree, or am I solo on this opinion?
> Andrew, isn't the real problem here that these methods should be on
> XhtmlRenderer or XhtmlUtils as their implementations are
> Xhtml-specific.  I don't have a problem per se with static methods on
> extendable classes.  However, if the class is going to contain
> extractable static convenience functions, then it is confusing to have
> both the class itself and a separate Utils class.  Given that we have a
> separate Utils class, these methods shouldn't be on the abstract class.
>
> -- Blake Sullivan
> >
> > -Andrew
>
>

Re: [Trinidad] CoreRenderer static methods

Posted by Blake Sullivan <bl...@oracle.com>.
Andrew Robinson wrote:
> There are static methods on CoreRenderer that I have no idea why they 
> are static. The two in particular that I noticed:
>
> static public void renderStyleClass
> static public void renderStyleClasses
>
> These are methods for renderers, why would they be static?
>
> There are some util style classes that are calling them, because of 
> this, I would think these methods should be moved into a Util class 
> and have the Renderer methods invoke the Util ones. IMO, static 
> utility functions do not belong on classes that are meant to be 
> extended. Do others agree, or am I solo on this opinion?
Andrew, isn't the real problem here that these methods should be on 
XhtmlRenderer or XhtmlUtils as their implementations are 
Xhtml-specific.  I don't have a problem per se with static methods on 
extendable classes.  However, if the class is going to contain 
extractable static convenience functions, then it is confusing to have 
both the class itself and a separate Utils class.  Given that we have a 
separate Utils class, these methods shouldn't be on the abstract class.

-- Blake Sullivan
>
> -Andrew