[XSL-LIST Mailing List Archive Home] [By Thread] [By Date]

Re: [xsl] Many sets of eyes ...


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>."&#x000A;</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>."&#xA;</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>."&#xA;</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>."&#xA;</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
Keywords