Subject: Re: [xsl] Many sets of eyes ... From: Jeni Tennison <jeni@xxxxxxxxxxxxxxxx> Date: Mon, 11 Feb 2002 19:47:01 +0000 |
Hi Curtis, > A few days ago I had a problem to solve and was tempted to take it > to the list but decided to grit my teeth and just get on with it. > I'm still pretty new to XSLT so I'm afraid I've probably made some > gaffes in my solution. I was hoping that the trained eyes on the > list (e.g. the Uber-Deities Jeni, Michael, Jim, et. al) can give me > some pointers on stylistic / performance issues? And of course help > me point out gaffes ;-) Looks great :) There are a couple of stylistic things that I'd do slightly differently. First, I'd use simply * rather than ./* - paths are always evaluated relative to the context node, so having ./ at the start of a path doesn't change what the path returns, just adds a couple of "line noise" characters (as the XQuery guys would apparently call them ;) to the expression. I'd also personally use [1] rather than [position() = 1] but that's just to avoid typing. More importantly from a performance point of view, I'd use xsl:choose where I could. You currently have: > <xsl:if test="name(following-sibling::*[position()=1]/*) = name(./*)"> > <xsl:text>, </xsl:text> > </xsl:if> > > <xsl:if test="name(following-sibling::*[position()=1]/*) != name(./*)"> > <xsl:text>."
</xsl:text> > </xsl:if> The two conditions are mutually exclusive - either one is true or the other is true. So there's no need to test both of them - instead you can just use an xsl:choose, with one xsl:when (and the test) and an xsl:otherwise to catch the case when the test isn't true: <xsl:choose> <xsl:when test="name(following-sibling::*[1]/*) = name(*)"> <xsl:text>, </xsl:text> </xsl:when> <xsl:otherwise>."
</xsl:otherwise> </xsl:choose> The other thing is that there's no need to test whether an element is there if all you're going to do if it *is* there is apply templates to it. You currently do: > <xsl:if test="joe"> > <xsl:apply-templates select="joe"/> > </xsl:if> > <xsl:if test="ann"> > <xsl:apply-templates select="ann"/> > </xsl:if> But if the current node doesn't have a joe element as a child then applying templates to all the joe element children won't do anything; similarly, if the current node doesn't have an ann element as a child then applying templates to all the ann element children won't do anything. So the above is equivalent to: <xsl:apply-templates select="joe" /> <xsl:apply-templates select="ann" /> Which is also equivalent to: <xsl:apply-templates select="joe | ann" /> Which is actually in this case equivalent to: <xsl:apply-templates select="*" /> since the wrap element doesn't contain elements other than joe or ann elements. Don't worry about the fact that you're not naming the elements that you're applying templates to in the xsl:apply-templates - the processor will sort out which of the templates it needs to use, from the match patterns. Oh, and I suppose that you could derive the "Joe says" or "Ann says" algorithmically from the name of the child element, by translating the first letter to a capital, with: <xsl:value-of select="concat(translate(substring(name(*), 1, 1), 'aj', 'AJ'), substring(name(*), 2))"> <xsl:text> says: "</xsl:text> It wouldn't win you anything on performance, I think, but it makes it a bit more extensible (especially if you use the full alphabet rather than just 'a' and 'j' when translating). The final thing is that rather than having a template matching the root element and iterating over the wrap elements with an xsl:for-each, I'd have a separate template for the wrap elements - that's just stylistic thing, really. Since you're stripping out whitespace, and not adding anything in the template for the root element, you can get rid of the template for the root element altogether. So my version would be: <xsl:template match="wrap"> <xsl:if test="name(preceding-sibling::*[1]/*) != name(*)"> <xsl:value-of select="concat(translate(substring(name(*), 1, 1), 'aj', 'AJ'), substring(name(*), 2))"> <xsl:text> says: "</xsl:text> </xsl:if> <xsl:apply-templates select="*" /> <xsl:choose> <xsl:when test="name(following-sibling::*[1]/*) = name(*)"> <xsl:text>, </xsl:text> </xsl:when> <xsl:otherwise>."
</xsl:otherwise> </xsl:choose> </xsl:template> <xsl:template match="joe"> <xsl:value-of select="."/> </xsl:template> <xsl:template match="ann"> <xsl:value-of select="."/> </xsl:template> --- Woo-hoo, a chance to use xsl:for-each-group from XSLT 2.0 that *doesn't* use group-by! :) To do the same thing in XSLT 2.0, you could do the following: <xsl:template match="root"> <xsl:for-each-group select="wrap" group-adjacent="name(*)" /> <xsl:value-of select="concat(translate(substring(name(*), 1, 1), 'aj', 'AJ'), substring(name(*), 2))"> <xsl:text> says: "</xsl:text> <xsl:for-each select="current-group()"> <xsl:apply-templates select="*" /> <xsl:if test="position() != last()">, </xsl:if> </xsl:for-each> <xsl:text>."
</xsl:text> </xsl:for-each-group> </xsl:template> <xsl:template match="joe"> <xsl:value-of select="."/> </xsl:template> <xsl:template match="ann"> <xsl:value-of select="."/> </xsl:template> --- I hope you find these comments helpful, Jeni --- Jeni Tennison http://www.jenitennison.com/ XSL-List info and archive: http://www.mulberrytech.com/xsl/xsl-list
Current Thread |
---|
|
<- Previous | Index | Next -> |
---|---|---|
[xsl] Many sets of eyes ..., Curtis Burisch | Thread | Re: [xsl] Many sets of eyes ..., Thomas B. Passin |
Re: [xsl] newbie question: nested t, Trevor Nash | Date | Re: [xsl] How to assign a value and, Maneshi Tuli |
Month |