You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flink.apache.org by Szabó Péter <ne...@gmail.com> on 2015/02/27 10:30:49 UTC
[DISCUSS] URI NullPointerException in TestBaseUtils
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
Re: [DISCUSS] URI NullPointerException in TestBaseUtils
Posted by Márton Balassi <ba...@gmail.com>.
+1
On Fri, Feb 27, 2015 at 11:32 AM, Szabó Péter <ne...@gmail.com>
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 <tr...@apache.org>:
>
> > 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 <nemderogatorius@gmail.com
> >
> > 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
> > >
> >
>
Re: [DISCUSS] URI NullPointerException in TestBaseUtils
Posted by Szabó Péter <ne...@gmail.com>.
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 <tr...@apache.org>:
> 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 <ne...@gmail.com>
> 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
> >
>
Re: [DISCUSS] URI NullPointerException in TestBaseUtils
Posted by Till Rohrmann <tr...@apache.org>.
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 <ne...@gmail.com>
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
>