What Simon proposed makes a lot of sense to me.
On Wed, Jul 2, 2008 at 1:18 PM, [EMAIL PROTECTED]
<[EMAIL PROTECTED]> 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
>
>
