jaxen
  1. jaxen
  2. JAXEN-153

jaxenException should not extend SAXPathException

    Details

    • Type: Improvement Improvement
    • Status: Open Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.1
    • Fix Version/s: 1.2
    • Component/s: core
    • Labels:
      None
    • Number of attachments :
      0

      Description

      Found this unresolved issue in my old e-mail. Adding here so we won't lose track of it again:

      Recently I noticed that JaxenException now extends SAXPathException. Does this make sense? It feels to me that maybe it should be the other way around, if these two classes have an inheritance relationship at all. Possibly they should each be independent classes. Ideally I'd like to hide the org.jaxen.saxpath classes completely from public view. However, this prevents that. Should a casual user of Jaxen (i.e. someone not implementing their own navigator class or using org.jaxen.saxpath directly) encounter SAXPathExceptions, or should JaxenException be sufficient?

      Compatibility wise the SAXPathException class has changed packages since 1.0, so changing the inheritance relationship would not be an additional incompatibility beyond what's already there.

      I plan to fix this in beta 11. If anyone would rather not see this change, comment here. Thanks.

        Activity

        Hide
        Elliotte Rusty Harold added a comment -

        I've taken my first crack at fixing this and all tests are passing. Furthermore, the SAXPath packages are now pretty effectively decoupled from the other jaxen packages. The jaxen packages are less coupled to the SAXPath packages. This prevents a casual user (i.e. someone not implementing a navigator) from encountering or worrying about SAXPath at all.

        There is one downside to this change. It breaks pretty much all existing navigators. The fix, however, is straight-forward. You just need to change "throws SAXPathException" to "throws JaxenException" on one method, parseXPath. No other changes should be needed in third party code. Still, this makes me a little nervous. Should we:

        1. Check the code in.
        2. Mark it Won't Fix and accept the confusing an irrational exception structure.
        3. Defer this to 1.2 or 2.0..

        Thoughts?

        Show
        Elliotte Rusty Harold added a comment - I've taken my first crack at fixing this and all tests are passing. Furthermore, the SAXPath packages are now pretty effectively decoupled from the other jaxen packages. The jaxen packages are less coupled to the SAXPath packages. This prevents a casual user (i.e. someone not implementing a navigator) from encountering or worrying about SAXPath at all. There is one downside to this change. It breaks pretty much all existing navigators. The fix, however, is straight-forward. You just need to change "throws SAXPathException" to "throws JaxenException" on one method, parseXPath. No other changes should be needed in third party code. Still, this makes me a little nervous. Should we: 1. Check the code in. 2. Mark it Won't Fix and accept the confusing an irrational exception structure. 3. Defer this to 1.2 or 2.0.. Thoughts?
        Hide
        peter royal added a comment -

        I say defer until 1.2/2.0 .. I'd rather not force this breaking change on users so late in a release cycle. It can be the first thing we do after shipping 1.1

        Show
        peter royal added a comment - I say defer until 1.2/2.0 .. I'd rather not force this breaking change on users so late in a release cycle. It can be the first thing we do after shipping 1.1
        Hide
        Elliotte Rusty Harold added a comment -

        Deferring to 1.2 per Peter

        Show
        Elliotte Rusty Harold added a comment - Deferring to 1.2 per Peter

          People

          • Assignee:
            Elliotte Rusty Harold
            Reporter:
            Elliotte Rusty Harold
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated: