Skip navigation
Up to Discussions in Tinder
5118 Views 7 Replies Latest reply: Oct 26, 2009 2:55 AM by Guus der Kinderen RSS
andi5 Bronze
andi5
10 posts since
Oct 12, 2009
Currently Being Moderated

Oct 12, 2009 10:13 AM

NPE in o.x.p.JID

In

 

public JID(String node, String domain, String resource, boolean skipStringprep)

 

the docs say that IllegalArgumentException is thrown if the JID is not valid.

Nonetheless, four lines after that

 

throw new NullPointerException("Domain cannot be null");

  • Guus der Kinderen KeyContributor
    Guus der Kinderen
    771 posts since
    Sep 8, 2005
    Currently Being Moderated
    Oct 13, 2009 12:50 PM (in response to andi5)
    Re: NPE in o.x.p.JID

    Individual parts of a JID can be null, except for the domain-part, which always has to be a non-null value. This is why a null-check is done on domain, before any of the validation takes place. If the provided argument are invalid (e.g: do not pass stringprep), IllegalArgumentExceptions are thrown. I've updated the javadoc to make mention of the possible NullPointer.

    • andi5 Bronze
      andi5
      10 posts since
      Oct 12, 2009
      Currently Being Moderated
      Oct 14, 2009 2:12 AM (in response to Guus der Kinderen)
      Re: NPE in o.x.p.JID

      Yep, but what would be wrong about throwing an IllegalArgumentException as well?  In the end, it is an illegal argument and that way I could simply catch one Exception instead of catching a superclass or having several catch blocks.

      • Guus der Kinderen KeyContributor
        Guus der Kinderen
        771 posts since
        Sep 8, 2005
        Currently Being Moderated
        Oct 14, 2009 2:24 AM (in response to andi5)
        Re: NPE in o.x.p.JID

        My biggest concern is that it'd break backwards compatibility. Although I do not dislike your idea of throwing the same exception for both types of problems (I agree with you that the current implementation is a bit messy), I don't want to risk breaking other projects over a relatively minor, mostly cosmetic change.

        • andi5 Bronze
          andi5
          10 posts since
          Oct 12, 2009
          Currently Being Moderated
          Oct 14, 2009 4:58 AM (in response to Guus der Kinderen)
          Re: NPE in o.x.p.JID

          Hi Guus,

           

          I understand that concern, but still think that Tinder should handle it (more) correctly. I doubt that there really is code that handles the NPE and IAE differently, but rather think it either catches both as RunTimeException or Exception, ignores both or handles both equally.  But I may be wrong, of course.  From the API perspective, I would argue that this is a bug fix given the Javadocs (as released).

           

          BTW, I even think you could add "throws IllegalArgumentException" to the method, even if it does not really change requirements for the caller (IMHO).

           

          With regard to cosmetic, consider me as a new tinder user. I do want to check whether the constructor accepted the parts for a jid, so I catch IAE and suddenly NPEs fly around. I think adding NPE to the docs makes it uglier...

          Thanks for considering :-)

          • Guus der Kinderen KeyContributor
            Guus der Kinderen
            771 posts since
            Sep 8, 2005
            Currently Being Moderated
            Oct 25, 2009 7:49 AM (in response to andi5)
            Re: NPE in o.x.p.JID

            Over the past few days, I've given this some thought. I didn't change my mind though. Each of the thrown exceptions has a very specific cause and, arguably, require a different way of handling:

             

            A thrown IllegalArgumentException is thrown if the provided arguments contain illegal characters or are otherwise incompatible with the XMPP specs. Typically, a user has entered incorrect data. These exceptions indicate a problem that should eventually be reported back to the end-user that provided the input data (probably using an XMPP error of type modify).

             

            A thrown NullPointerException indicates a bug: somewhere, code allows a JID to be constructed without a domain. These kind of problems are typically not triggered by end users, but by the code processing or parsing data. These exceptions indicate a problem that should be reported back to the maintainers of the software and/or environment (note that end-users should be warned too, if appropiate. I'd expect errors with internal-server-error conditions here).

            • andi5 Bronze
              andi5
              10 posts since
              Oct 12, 2009
              Currently Being Moderated
              Oct 26, 2009 2:50 AM (in response to Guus der Kinderen)
              Re: NPE in o.x.p.JID

              Hey Guus,

              thanks for giving it a chance :-) Maybe it is a matter of choice in the end, so I like a definite answer and hope that you have come to conclusions that help in other areas of the library as well.

              Ciao,

              -- andi5

              • Guus der Kinderen KeyContributor
                Guus der Kinderen
                771 posts since
                Sep 8, 2005
                Currently Being Moderated
                Oct 26, 2009 2:55 AM (in response to andi5)
                Re: NPE in o.x.p.JID

                We will keep the code as-is. If you have suggestion on how to make it clearer for a developer through javadoc or other modifications (you're working from that perspective which makes you an expert ), let me know.

More Like This

  • Retrieving data ...

Bookmarked By (0)

Legend

  • Correct Answers - 10 points
  • Helpful Answers - 5 points