Issue 478: Throw an IllegalArgumentException when the folder already exists #479
Conversation
Changed the code so it behaves like it is described in the documentation.
| @@ -89,7 +89,9 @@ public File newFolder(String... folderNames) { | |||
| File file= getRoot(); | |||
| for (String folderName : folderNames) { | |||
| file= new File(file, folderName); | |||
| file.mkdir(); | |||
| if (!file.mkdir()) | |||
| throw new IllegalStateException( | |||
There was a problem hiding this comment.
I think IllegalArgumentException would be a better than IllegalStateException, but I wonder if we should simply throw an IOException
There was a problem hiding this comment.
The reason why I used IllegalStateException is to be consistent. An IllegalStateException is also thrown if for some reason a file could not be created. (line 64)
There was a problem hiding this comment.
The change to add the IllegalStateException was added after the 4.10 release, so we can fix it now without breaking anyone.
kcooney suggested some changes for junit-team#479
After solving junit-team#479 also fixed the indenting
| @@ -61,7 +61,7 @@ public void create() throws IOException { | |||
| public File newFile(String fileName) throws IOException { | |||
| File file= new File(getRoot(), fileName); | |||
| if (!file.createNewFile()) | |||
| throw new IllegalStateException( | |||
| throw new IllegalArgumentException( | |||
There was a problem hiding this comment.
It looks like IOException would be consistent with what JDK7 would do. See http://tinyurl.com/8bkvws7 (createFile() throws a subclass of IOException if the file exists)
|
Looks good. Thanks. Let's go with IOException, since that's what the JDK appears to be settling on. |
It's more consistent to throw an IOException instead of an IllegalArgumentException
|
Thanks! |
Issue 478: Throw an IllegalArgumentException when the folder already exists
Changed the code so it behaves like it is described in the documentation for #478.