[DISCUSS] URI NullPointerException in TestBaseUtils

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[DISCUSS] URI NullPointerException in TestBaseUtils

Szabó Péter
The following code snippet in from TestBaseUtils:

protected static File asFile(String path) {
   try {
      URI uri = new URI(path);
      if (uri.getScheme().equals("file")) {
         return new File(uri.getPath());
      } else {
         throw new IllegalArgumentException("This path does not denote a
local file.");
      }
   } catch (URISyntaxException e) {
      throw new IllegalArgumentException("This path does not describe a
valid local file URI.");
   }
}

If uri does not have a scheme (e.g. "/home/something.txt"),
uri.getScheme().equals("file") throws a NullPointerException instead of an
IllegalArgumentException is thrown. I feel it would make more sense to
catch the NullPointerException at the end.

What do you guys think?

Peter
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] URI NullPointerException in TestBaseUtils

Till Rohrmann
Catching the NullPointerException and throwing an IllegalArgumentException
with a meaningful message might clarify things.

Considering that it only affects the TestBaseUtils, it should not be big
deal to change it.

On Fri, Feb 27, 2015 at 10:30 AM, Szabó Péter <[hidden email]>
wrote:

> The following code snippet in from TestBaseUtils:
>
> protected static File asFile(String path) {
>    try {
>       URI uri = new URI(path);
>       if (uri.getScheme().equals("file")) {
>          return new File(uri.getPath());
>       } else {
>          throw new IllegalArgumentException("This path does not denote a
> local file.");
>       }
>    } catch (URISyntaxException e) {
>       throw new IllegalArgumentException("This path does not describe a
> valid local file URI.");
>    }
> }
>
> If uri does not have a scheme (e.g. "/home/something.txt"),
> uri.getScheme().equals("file") throws a NullPointerException instead of an
> IllegalArgumentException is thrown. I feel it would make more sense to
> catch the NullPointerException at the end.
>
> What do you guys think?
>
> Peter
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] URI NullPointerException in TestBaseUtils

Szabó Péter
Yeah, I agree, it is at best a cosmetic issue. I just wanted to let you
know about it.

Peter


2015-02-27 11:10 GMT+01:00 Till Rohrmann <[hidden email]>:

> Catching the NullPointerException and throwing an IllegalArgumentException
> with a meaningful message might clarify things.
>
> Considering that it only affects the TestBaseUtils, it should not be big
> deal to change it.
>
> On Fri, Feb 27, 2015 at 10:30 AM, Szabó Péter <[hidden email]>
> wrote:
>
> > The following code snippet in from TestBaseUtils:
> >
> > protected static File asFile(String path) {
> >    try {
> >       URI uri = new URI(path);
> >       if (uri.getScheme().equals("file")) {
> >          return new File(uri.getPath());
> >       } else {
> >          throw new IllegalArgumentException("This path does not denote a
> > local file.");
> >       }
> >    } catch (URISyntaxException e) {
> >       throw new IllegalArgumentException("This path does not describe a
> > valid local file URI.");
> >    }
> > }
> >
> > If uri does not have a scheme (e.g. "/home/something.txt"),
> > uri.getScheme().equals("file") throws a NullPointerException instead of
> an
> > IllegalArgumentException is thrown. I feel it would make more sense to
> > catch the NullPointerException at the end.
> >
> > What do you guys think?
> >
> > Peter
> >
>
Reply | Threaded
Open this post in threaded view
|

Re: [DISCUSS] URI NullPointerException in TestBaseUtils

Márton Balassi
+1

On Fri, Feb 27, 2015 at 11:32 AM, Szabó Péter <[hidden email]>
wrote:

> Yeah, I agree, it is at best a cosmetic issue. I just wanted to let you
> know about it.
>
> Peter
>
>
> 2015-02-27 11:10 GMT+01:00 Till Rohrmann <[hidden email]>:
>
> > Catching the NullPointerException and throwing an
> IllegalArgumentException
> > with a meaningful message might clarify things.
> >
> > Considering that it only affects the TestBaseUtils, it should not be big
> > deal to change it.
> >
> > On Fri, Feb 27, 2015 at 10:30 AM, Szabó Péter <[hidden email]
> >
> > wrote:
> >
> > > The following code snippet in from TestBaseUtils:
> > >
> > > protected static File asFile(String path) {
> > >    try {
> > >       URI uri = new URI(path);
> > >       if (uri.getScheme().equals("file")) {
> > >          return new File(uri.getPath());
> > >       } else {
> > >          throw new IllegalArgumentException("This path does not denote
> a
> > > local file.");
> > >       }
> > >    } catch (URISyntaxException e) {
> > >       throw new IllegalArgumentException("This path does not describe a
> > > valid local file URI.");
> > >    }
> > > }
> > >
> > > If uri does not have a scheme (e.g. "/home/something.txt"),
> > > uri.getScheme().equals("file") throws a NullPointerException instead of
> > an
> > > IllegalArgumentException is thrown. I feel it would make more sense to
> > > catch the NullPointerException at the end.
> > >
> > > What do you guys think?
> > >
> > > Peter
> > >
> >
>