Uploaded image for project: 'Commons JXPath'
  1. Commons JXPath
  2. JXPATH-59

use SKIP instead of SPECIAL_TOKEN in parser?

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.1 Final
    • Fix Version/s: None
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Bugzilla Id:
      18284

      Description

      There may be a good reason why this wasn't already done, but since I couldn't
      see one right off and since you can always reject this e.r., I'm logging it anyway.

      The XPath.jj file from with JXPath's parser is generated includes this in its
      lexical spec:

      /----------------------------/
      /* Skip Whitespace everywhere */
      /----------------------------/

      /* [39] ExprWhitespace ::= S */
      SPECIAL_TOKEN :
      {
      <S: ( [" ", "\t", "\n", "\r"] )+>
      }

      Given that the <S> token is never used, why not use SKIP instead?

      SKIP :
      {
      [" ", "\t", "\n" , "\r", "\f"] // note addition of "\f"
      }

      This has a couple of advantages: (1) it eliminates the need for the lexer to
      create an instance of Token that gets attached as a "special" to real tokens you
      care about (since you don't seem to be using the <S> token, why create it?); (2)
      using a simple character class is slightly faster than a character class with a
      kleene + around it.

      I didn't look very closely, so perhaps there's a place where the <S> token is
      used after all. If so, you can disregard this suggestion.

        Activity

        Hide
        dmitri@plotnix.com Dmitri Plotnikov added a comment -

        Done. Thanks for the report.

        Show
        dmitri@plotnix.com Dmitri Plotnikov added a comment - Done. Thanks for the report.

          People

          • Assignee:
            Unassigned
            Reporter:
            eric.d.friedman@wellsfargo.com Eric Friedman
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development