myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Leonardo Uribe" <lu4...@gmail.com>
Subject Re: [VOTE] Upgrade s:limitRendered to tomahawk
Date Thu, 03 Jul 2008 21:44:02 GMT
On Thu, Jul 3, 2008 at 4:31 PM, simon <simon.kitching@chello.at> wrote:

> The name t:renderOne sounds good to me.
>

Sounds good to me too. My intention of the next days is propose several
components to be
upgraded, in order to check what we need to do (more than upgrade components
is the discussion
involved and the final todos list). Only non dojo components will be
proposed.


>
> On Thu, 2008-07-03 at 10:07 -0600, Andrew Robinson wrote:
> > If that were done, I would recommend the name "renderOne". choose in
> > JSF sounds closer to the select* prefix, reserved for UIInput
> > components. renderOne is clear in that it shows that it affects
> > rendering, where choose does not define what it does (one could argue
> > that it was a bad naming decision for JSTL and XSLT). Choose to me
> > sounds more like a SELECT component than a choice to only render one.
> > But that is my opinion.
> >
> > -Andrew
> >
> > On Wed, Jul 2, 2008 at 3:58 PM, Mike Kienenberger <mkienenb@gmail.com>
> wrote:
> > > What Simon proposed makes a lot of sense to me.
> > >
> > >
> > > On Wed, Jul 2, 2008 at 1:18 PM, simon.kitching@chello.at
> > > <simon.kitching@chello.at> wrote:
> > >> Andrew Robinson schrieb:
> > >>>>
> > >>>> You must have had a real use case that pushed you to write this
> > >>>> component.
> > >>>> Can you please describe it?
> > >>>>
> > >>>
> > >>> The same as all usages of <c:choose />. The index based or more
than
> > >>> one are just added benefits I threw in. I can provide examples, but
I
> > >>> shouldn't have to.
> > >>
> > >> I certainly think all new components should have to provide proper
> > >> use-cases. Having very rarely used components in Tomahawk:
> > >> * makes it hard for users to find what they want (steeper learning
> curve)
> > >> * increases the maintenance burden
> > >> * increases the jarfile size
> > >>
> > >> So components should only go in if they are useful to a reasonable
> number of
> > >> people.
> > >>
> > >>>  Just because someone can't think of when it would
> > >>> be needed, doesn't mean it never would be useful, but I can appease
> > >>> you curiosity.
> > >>
> > >> It's not curiosity. There is a vast amount of crap in Tomahawk right
> now, to
> > >> the point where Tomahawk is close to dying. There hasn't been a
> release for
> > >> a year. The number of open bugs is vast. So everyone *should* be
> watching
> > >> carefully to see that we don't increase the problems.
> > >>
> > >> That doesn't mean that good components cannot be added. But new stuff
> does
> > >> need to be evaluated for usefulness.
> > >>
> > >> And the author of a component is often too close to the code to see
> whether
> > >> it can be improved (that applies equally to me). Having other people
> look
> > >> critically at the purpose and API is very useful. You are free to
> point out
> > >> any issues with components I write (eg Orchestra stuff).
> > >>
> > >>>  I created the component so that people would stop using
> > >>> c:choose and c:if in JSF pages and complain that they don't work in
> > >>> tables and such.
> > >>>
> > >>> 1) default c:choose functionality (render the first match):
> > >>>
> > >>> <s:limitRendered>
> > >>>  <h:outputText value="#{person.first} #{person.last}"
> > >>> rendered="#{prefs.firstThenLast}" />
> > >>>  <h:outputText value="#{person.last}, #{person.first}"
> > >>> rendered="#{prefs.firstThenLast}" />
> > >>> </s:limitRendered>
> > >>>
> > >>
> > >> Yep, this is a good use case. As you demonstrate later in your email,
> > >> writing mutually-exclusive rendered expressions for a set of
> components can
> > >> get nasty.
> > >>
> > >> I'm not a JSTL user, so your reference to c:choose wasn't perhaps as
> clear
> > >> to me as it might be to others. I think this way:
> > >>
> > >> if (cond1)  render component 1
> > >> else if (cond2) render component 2
> > >> else if (cond3) render component 3
> > >> else render component 4
> > >>
> > >> Expanding the javadoc for the component a bit would be good,
> mentioning that
> > >> it simplifies rendered expressions for mutually-exclusive components.
> The
> > >> current docs don't mention that the implicit condition associated with
> the
> > >> "choose clauses" is the rendered expression; it makes sense once I
> know what
> > >> the component is doing but wasn't obvious at first.
> > >>
> > >>> 2) render index based. This behaves much like the tr:switcher
> > >>> component. But instead of using facets and facet names, it uses
> > >>>
> > >>> <s:limitRendered value="#{wizard.currentStep}" type="index">
> > >>>  <h:panelGroup>
> > >>>    <h:outputText value="This is wizard step 1" />
> > >>>  </h:panelGroup>
> > >>>  <h:panelGroup>
> > >>>    <h:outputText value="This is wizard step 2" />
> > >>>  </h:panelGroup>
> > >>>  <h:panelGroup>
> > >>>    <h:outputText value="This is wizard step 3" />
> > >>>  </h:panelGroup>
> > >>> </s:limitRendered>
> > >>>
> > >>
> > >> I'm not so sure about this. The tr:switcher makes sense to me; it
> chooses a
> > >> component to render by name which will not be easily broken by page
> changes,
> > >> and where the link between what the backing bean EL expression returns
> and
> > >> what facet is selected is clear (the name matches).
> > >>
> > >> Selecting by index has a far smaller set of use-cases I think. And it
> can
> > >> promote code fragility; coupling an index returned by the backing bean
> with
> > >> an array defined in the page has potential for trouble. But the wizard
> > >> use-case is an example of a valid use of this functionality.
> > >>
> > >>> 3) render multiple children. Can be used much like "-v" vs "-vvvv"
> can
> > >>> be used for command line verbosity
> > >>>
> > >>> <s:limitRendered value="#{verbosity}">
> > >>>  <h:outputText value="#{title}" />
> > >>>  <h:outputText value="#{usage}" />
> > >>>  <h:outputText value="#{description}" />
> > >>> </s:limitRendered>
> > >>>
> > >>
> > >> Equivalent to this:
> > >>  <h:outputText value="#{title}" rendered="#{verbosity >=1}"/>
> > >>  <h:outputText value="#{usage}" rendered="#{verbosity >=2}"/>
> > >>  <h:outputText value="#{description}" rendered="#{verbosity >=3}"/>
> > >>
> > >> Yes, the limitRendered approach is a little more efficient; only one
> EL
> > >> expression evaluated rather than 3. But any JSF developer understands
> the
> > >> latter, while no-one can understand the limitRendered approach without
> > >> looking up the docs first. And a pretty rare use case I would think.
> Worth
> > >> including perhaps if it didn't have any other negatives, but I think
> it
> > >> does: it forces the name of the component to be generic and the
> > >> documentation to be complex.
> > >>
> > >>> Now I cannot tell you all the reasons they may be useful, but I can
> > >>> say that many time in Trinidad authors chose to only provide
> > >>> functionality that they themselves could think of, making the
> > >>> component useless for every use case they could not think of. Perhaps
> > >>> I cannot think of great reasons to render more than one child at the
> > >>> moment, but who is to say no one will ever want that?
> > >>>
> > >>
> > >> Making functionality more generic can be good. But it also increases
> the
> > >> complexity of the learning curve. And it can force code to use generic
> > >> unhelpful names because there is no simple name that summarizes the
> > >> functionality.
> > >>
> > >> If just the "render only first rendered child" functionality is
> provided,
> > >> then the component could be called "t:choose" or "t:chooseOne"
> Wouldn't that
> > >> be easier for people to understand than "t:limitRendered"?
> > >>
> > >> If the "render child selected by index" is considered important to
> include,
> > >> then t:chooseOne could also be the name. It suggests similarity with
> > >> "c:choose", but with a little extra.
> > >>
> > >>> The main use case is to stop this code:
> > >>>
> > >>> <h:outputText value="a" rendered="#{value1 eq 'a'}" />
> > >>> <h:outputText value="b" rendered="#{value1 ne 'a' and value2 eq
'b'}"
> />
> > >>> <h:outputText value="c" rendered="#{value1 ne 'a' and value2 ne
'b'
> > >>> and value3 eq 'c'}" />
> > >>> <h:outputText value="otherwise" rendered="#{value1 ne 'a' and value2
> > >>> ne 'b' and value3 ne 'c'}" />
> > >>> etc.
> > >>>
> > >>> Several times I have had the use case where I only want to render a
> > >>> component if the previous one was not rendered. To get that
> > >>> functionality, I always had to negate the test of the rendered of the
> > >>> previous components then include the test for the current component.
> > >>>
> > >>> This is much easier to write and definitely to maintain:
> > >>>
> > >>> <s:limitRendered>
> > >>>  <h:outputText value="a" rendered="#{value1 eq 'a'}" />
> > >>>  <h:outputText value="b" rendered="#{value2 eq 'b'}" />
> > >>>  <h:outputText value="c" rendered="#{value3 eq 'c'}" />
> > >>>  <h:outputText value="otherwise" />
> > >>> </s:limitRendered>
> > >>>
> > >>> So for the most part, every use case in JSP to use <c:choose />
is a
> > >>> use case to use limitRendered in JSF. As mentioned, the index based
> or
> > >>> multiple support was just added functionality for rare use cases.
> > >>>
> > >>
> > >> Yep, agreed.
> > >>
> > >> My suggestion would be to promote this component, but in modified
> form:
> > >> * ditch the ability to select the first N
> > >> * ditch the ability to select a set of arbitrary elements by index
> (1,4,6)
> > >> * rename to t:chooseOne
> > >> * In normal case, choose "first rendered child"
> > >> * If an "index" attribute is defined, select child by index:
> > >>   <t:chooseOne index="#{elexpr}">
> > >>
> > >> Why?
> > >> * chooseOne is clear to understand, both for people used to JSTL and
> XSLT,
> > >> and others.
> > >> * the selecting of the first N elements is only very rarely useful,
> and the
> > >> selecting of arbitrary elements by index is both rare and actively
> dangerous
> > >> as it promotes fragile code. But it makes the helpful name "chooseOne"
> > >> impossible. Removing this functionality also simplifies the component
> > >> implementation.
> > >>
> > >> Regards,
> > >> Simon
> > >>
> > >>
> > >
>
>

Mime
View raw message