It is well-known that code inside a conditional operator *increases *the
cyclomatic complexity.
Thus we see many sources that recommend to avoid or refactor conditional
statements -- in many different languages.
I believe XSLT is no exception.
Just for example, here is a recommendation how to refactor Javascript code:
https://webuniverse.io/cyclomatic-complexity-refactoring-tips/
and inside it:
Refactoring a switch-like logic
Here is another example -- the Pluralsight course "Refactoring for C#
Developers"
https://www.pluralsight.com/library/courses/refactoring-csharp-developers/table-of-contents
<https://app.pluralsight.com/library/courses/refactoring-csharp-developers/table-of-contents>
.
In the expanded table of contents, under "Code Smells" we see:
Smell: Switch Statements
In the "method Code Smells module we see:
Smell: Conditional complexity
I saw a phrase on a candy cover: "The magic is in the mess" ...
Let us not create "magic" code, please
Cheers,
Dimitre
On Thu, Dec 5, 2019 at 8:34 AM John Lumley john(_at_)saxonica(_dot_)com <
xsl-list-service(_at_)lists(_dot_)mulberrytech(_dot_)com> wrote:
On 05/12/2019 14:51, Eliot Kimber ekimber(_at_)contrext(_dot_)com wrote:
Not really about modes, but I would replace the choice that acts on different
@val values with templates applied to the @val attribute, i.e.:
<xsl:template match="a">
<val><xsl:apply-templates select="@val"/></val>
</xsl:template>
<xsl:template match="@val[. ge 0]">
<xsl:value-of select="@val || ': positive'"/>
</xsl:template>
<xsl:template match="@val[. lt 0]">
<xsl:value-of select="@val || ': negative"/>
</xsl:template>
Note that I handle the bug in the original in that it would produce no result
when @val is "0" (zero).
The use of templates rather than xsl:choose makes the code cleaner, I think,
puts the focus at the template level on the @val attribute, which is the
focus of the business logic, and enables extension and override. For example,
if you want a value of exactly zero to have a different result, you could do
that by adding an override template. The original use of xsl:choose would
require overriding the entire template for the <a> element.
I'm of a different opinion, somewhat in line with some remarks Mike made
earlier. (Note that this topic can sometimes get a little 'heated' in a
gentle XSLT manner.)
The design issues in part depend on the 'scope' and 'size' of the
operation you're invoking and the likelihood that you'll need to 'add
additional semantics' at a later date or by overriding with 'library'
importation. Lets look at a slight rewriting of the 'choose' method in
XSLT3:
<xsl:template match="a" expand-text="true">
<val>
<xsl:choose>
<xsl:when test="@val ge 0">{@val}: positive</xsl:when>
<xsl:when test="@val lt 0">{@val}: negative</xsl:when>
</xsl:choose>
</val>
</xsl:template>
(Yes it could be terser or it could be conditional XPath, but bear with
me.) The point of the choose over the template is that the *entirety *of
the operation choice semantics (identifying the signum of a/@val) is
contained in a single place, in fact inside a single XML element, and won't
be effected by any other additional code *outside* this segment. You're
pretty safe in the knowledge that if something's going wrong (e.g. the zero
case Eliot pointed out and another possible fault described later), then
the fault should *only be within this code section, *nowhere else. And
choose also has a totally defined order of checking - each when is tested
in turn, so you can control accurately the sequence of checks.
If that is what you wanted (contained scope), then if you used templates,
even moded ones, an additional matching template, almost anywhere else in
your stylesheets, might confound your nice solution. For example:
<xsl:template match="a/@val[. mod 2 eq 0]" <a/@val[.mod2eq0]>
priority="2">{@val}: even</xsl:template>
would alter the result. And similarly the 'default priority order' between
template rules is sometimes not as straightforward as it seems. Believe me,
with large stylesheets spread over many files it's extremely easy to find
that an errant template somewhere in the back of beyond is trumping the
code sections you're tearing your hair out debugging. And with stylesheet
importation this becomes even more prevalent as importation precedence wins
out over priority every time. This means that for example if your
stylesheet was imported into another, which contained:
<xsl:template match="a/@val[. eq 0]" <a/@val[.eq0]>><zero/></xsl:template>
then your design will have been 'broken' (perhaps unintentionally) by
someone else - the designer of the importing stylesheet. (Currently there
are no mechanisms, outside use of packages, to make templates local, even
as children of a mode, so all are effectively global in scope.)
But if you do expect to have a large and variable set of conditions that
in different cases will supercede each other, such as for example in the
design of DITA-OT, then templates are certainly the way. And if you're
using next-match to pre/post-process special cases, then you will need to
use templates.
Note that in this particular example lurks a perhaps more insidious design
issue for which a choose may have advantage. What if a/@val="abc", assuming
no schema-awareness checking for integer values. With choose an error would
be thrown during evaluation of the first when/@test ('abc' cannot be cast
to an integer); in the template case nothing would happen - errors in
patterns mean 'no match' and the usual default behaviour for @* is never
matching: hence the result would be just <val/> and you might be left
scratching your head.
For me the choices really come down to:
- How local is this operation? How many separable sections? How large
are they?
- Are some parts likely to be used from another section of the program?
- Do I expect to modify it, or update it, or override it?
etc... For some (most?) templates are the answer, for others choose wins
out. The most interesting are of course a toss-up!
--
*John Lumley* MA PhD CEng FIEE
john(_at_)saxonica(_dot_)com
on behalf of Saxonica Ltd
XSL-List info and archive <http://www.mulberrytech.com/xsl/xsl-list>
EasyUnsubscribe <http://lists.mulberrytech.com/unsub/xsl-list/782854> (by
email <>)
--~----------------------------------------------------------------
XSL-List info and archive: http://www.mulberrytech.com/xsl/xsl-list
EasyUnsubscribe: http://lists.mulberrytech.com/unsub/xsl-list/1167547
or by email: xsl-list-unsub(_at_)lists(_dot_)mulberrytech(_dot_)com
--~--