overly complex XPath logic

[sydb]

The "spec-in-module" constraint in att.identified currently looks like this (line numbers added):

[01]  <constraint>
[02]    <sch:rule context="tei:elementSpec[ @module ]
[03]                      |tei:classSpec[ @module ]
[04]                      |tei:macroSpec[ @module ] ">
[05]      <sch:assert test="( not( ancestor::tei:schemaSpec | ancestor::tei:TEI | ancestor::tei:teiCorpus ) )
[06]                        or
[07]                        (
[08]                          not( @module )
[09]                          or
[10]                          (
[11]                            not( //tei:moduleSpec )
[12]                            and
[13]                            not( //tei:moduleRef )
[14]                          )
[15]                          or
[16]                          ( //tei:moduleSpec[ @ident = current()/@module ] )
[17]                          or
[18]                          ( //tei:moduleRef[ @key = current()/@module ] )
[19]                        )">
[20]      Specification <sch:value-of select="@ident"/>: the value of
[21]      the module attribute ("<sch:value-of select="@module"/>")
[22]      should correspond to an existing module, via a moduleSpec or
[23]      moduleRef</sch:assert>
[24]    </sch:rule>
[25]  </constraint>

This test, which was last worked on > 13 years ago by SPQR, strikes me as overly complex.

First, the overall structure is needlessly complicated with at least one extra level of parens. If we replace each actual XPath snippet with the line number it is on, the structure boils down to:

[05]
or
(
  [08]
  or
  (
    [11]
    and
    [13]
  )
  or
  [16]
  or
  [18]
)

Which might be easier to grok with less whitespace:

[05] or ( [08] or ( [11] and [13] ) or [16] or [18] )

If we replace the compound ( [11] and [13] ) with [12], we get

[05] or ( [08] or [12] or [16] or [18] )

I submit that the parens add nothing (but confusion) to the logic of the test.

If we examine the structure of [12] more closely, it is ( not(A) and not(B) ). Logically that is precisely euqivalent to not( A or B ).

Furthermore, line 08 (and thus 09) is (are) completely reduntant: the rule is only being fired when a @module attribute is present.

And lastly, rather than test the values of @ident of <moduleSpec> separately form the values of @key of <moduleRef>, why not test them at the same time?

Thus I think the entire thing should be reduced to:

    <!--
      The @test below comprises three expressions in a disjunction.
      Only if all three are false() is the error message generated;
      i.e., if any one of the three is true() no message is
      generated. The first two test for whether or not we are in a
      circumstance in which the basic question, “does the module
      @module exist?” can be meaningfully asked. The sub-tests are:
      1) This specification element (which has a @module attr) is free-standing; i.e. it is NOT a descendant of <schemaSpec>, <TEI>, nor of <teiCorpus> — this is TRUE for the individual files in which we store the TEI P5 specifications, but false for p5.xml (or p5subset.xml) itself, and false for any proper customization file.
      2) There are no <moduleSpec> nor <moduleRef> elements in this file.
      3) In this file there is either a <moduleSpec>’s @ident or a <moduleRef>’s @key which matches this specification’s @module.
    -->
    <constraint>
      <sch:rule context="tei:elementSpec[ @module ]
                        |tei:classSpec[ @module ]
                        |tei:macroSpec[ @module ] ">
        <sch:assert test="( not( ancestor::tei:schemaSpec | ancestor::tei:TEI | ancestor::tei:teiCorpus ) )
                          or
                          not( //tei:moduleSpec | //tei:moduleRef )
                          or
                          ( //tei:moduleSpec/@ident | //tei:moduleRef/@key ) = current()/@module
                          ">
        Specification <sch:value-of select="@ident"/>: the value of
        the module attribute ("<sch:value-of select="@module"/>")
        should correspond to an existing module, via a moduleSpec or
        moduleRef</sch:assert>
      </sch:rule>
    </constraint>

I have tested this only insofar as I know that with this change the Guidelines will build and the Schematron generated is valid. I have NOT checked that the Schematron generated actually performs the test correctly. But that said, I am not sure that we know it currently performs the test correctly.

P.S. Although I admit that there are folks that would find the third clause easier to read & understand if it were written as follows, I am not one of them, and it would be quite a bit longer.

some $moduleName in //tei:moduleSpec/@ident | //tei:moduleRef/@key satisfies $moduleName eq @module