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 |
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 > |
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 > > > |
+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 > > > > > > |
Free forum by Nabble | Edit this page |