You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by krichter722 <gi...@git.apache.org> on 2018/05/21 03:14:32 UTC
[GitHub] commons-fileupload pull request #13: FileItem.java: remove throwing of Excep...
GitHub user krichter722 opened a pull request:
https://github.com/apache/commons-fileupload/pull/13
FileItem.java: remove throwing of Exception
Indicating that an interface method throws Exception is overly broad and
hides design issues after inheritance changes.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/krichter722/commons-fileupload throws-exception
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/commons-fileupload/pull/13.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #13
----
commit 77d68a7ce0d15c05d93ef56f8bd28d0cd454a131
Author: Karl-Philipp Richter <kr...@...>
Date: 2018-05-21T01:20:47Z
FileItem.java: remove throwing of Exception
Indicating that an interface method throws Exception is overly broad and
hides design issues after inheritance changes.
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-fileupload issue #13: FileItem.java: remove throwing of Exception
Posted by krichter722 <gi...@git.apache.org>.
Github user krichter722 commented on the issue:
https://github.com/apache/commons-fileupload/pull/13
In any previous situation users needed to catch `Exception`. Since both exception extend `Exception` there's no case where they need to change any code.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-fileupload issue #13: FileItem.java: remove throwing of Exception
Posted by sebbASF <gi...@git.apache.org>.
Github user sebbASF commented on the issue:
https://github.com/apache/commons-fileupload/pull/13
FooDiskItem.write is not part of FileUpload, so it *can* throw Exception if it has not been recompiled since it was compiled against FileUpload 1.3.3.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-fileupload issue #13: FileItem.java: remove throwing of Exception
Posted by garydgregory <gi...@git.apache.org>.
Github user garydgregory commented on the issue:
https://github.com/apache/commons-fileupload/pull/13
I think that would break binary compatibility. So it would have to be for 2.0.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-fileupload issue #13: FileItem.java: remove throwing of Exception
Posted by krichter722 <gi...@git.apache.org>.
Github user krichter722 commented on the issue:
https://github.com/apache/commons-fileupload/pull/13
> True, but what if I have a subclass of DiskFileItem that throws an Exception? What happens at runtime?
Correct, I didn't think about that. I'm already glad, you're willing to change this, so just put it into the next major release.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-fileupload issue #13: FileItem.java: remove throwing of Exception
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/commons-fileupload/pull/13
[](https://coveralls.io/builds/17079215)
Coverage remained the same at 77.177% when pulling **77d68a7ce0d15c05d93ef56f8bd28d0cd454a131 on krichter722:throws-exception** into **fc2818a419e929f60f2288898a9c815aee9e8261 on apache:master**.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-fileupload issue #13: FileItem.java: remove throwing of Exception
Posted by sebbASF <gi...@git.apache.org>.
Github user sebbASF commented on the issue:
https://github.com/apache/commons-fileupload/pull/13
AFAICT this only affects source compatibility.
I was able to run the subclass (throwing Exception) against the base class (updated to throw IOE).
Of course I could not compile the subclass without adjusting the throws clause.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-fileupload issue #13: FileItem.java: remove throwing of Exception
Posted by sebbASF <gi...@git.apache.org>.
Github user sebbASF commented on the issue:
https://github.com/apache/commons-fileupload/pull/13
The throws clause is not part of the method signature, and does not affect binary compatibility.
However it does affect source compatibilty
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-fileupload issue #13: FileItem.java: remove throwing of Exception
Posted by sebbASF <gi...@git.apache.org>.
Github user sebbASF commented on the issue:
https://github.com/apache/commons-fileupload/pull/13
@krichter722 Yes. I meant to say: changing a throw clause can affect source compatibility
@garydgregory - are you asking whether an overloaded method will have to be amended?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-fileupload issue #13: FileItem.java: remove throwing of Exception
Posted by garydgregory <gi...@git.apache.org>.
Github user garydgregory commented on the issue:
https://github.com/apache/commons-fileupload/pull/13
I am concerned about:
- I create a class or jar with a subclass of `DiskFileItem` called `FooDiskFileItem` that throws an `Exception` against FileUpload 1.3.3.
- We release FileUpload 1.4.0 with a `write()` that now throws `IOException`.
- I write code against 1.4.0 to catch IOException.
- I run my `FooDiskFileItem` that throws an `Exception` (not an `IOException`).
- My new code obviously does not catch Exception and it percolates up the stack.
- This is OK, since I am writing new code, if I want to use old `DiskFileItem` classes, I catch Exception.
Does that make sense? Is that OK?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-fileupload issue #13: FileItem.java: remove throwing of Exception
Posted by garydgregory <gi...@git.apache.org>.
Github user garydgregory commented on the issue:
https://github.com/apache/commons-fileupload/pull/13
True, but what if I have a subclass of DiskFileItem that throws an Exception?
Note: We are OK with breaking source compatibility but not BC outside of a major release.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-fileupload issue #13: FileItem.java: remove throwing of Exception
Posted by jochenw <gi...@git.apache.org>.
Github user jochenw commented on the issue:
https://github.com/apache/commons-fileupload/pull/13
I agree with Garys concerns (in particular, because I know how widely spread fileupload is, and how difficult it can be to replace the containers version with your own), so won't change this. The issue is a technicality, and not a real problem. Sorry!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-fileupload issue #13: FileItem.java: remove throwing of Exception
Posted by sebbASF <gi...@git.apache.org>.
Github user sebbASF commented on the issue:
https://github.com/apache/commons-fileupload/pull/13
Surely if your new code uses FooDiskFileItem, it needs to know that it throws Exception and catch that accordingly? Why would you knowingly use FooDiskFileItem but not catch Exception?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
[GitHub] commons-fileupload issue #13: FileItem.java: remove throwing of Exception
Posted by krichter722 <gi...@git.apache.org>.
Github user krichter722 commented on the issue:
https://github.com/apache/commons-fileupload/pull/13
> I run my FooDiskFileItem that throws an Exception (not an IOException).
How? If `DiskItem.write` doesn't throw `Exception` `FooDiskItem.write` can't.
> My new code obviously does not catch that Exception and it percolates up the stack.
That's what you want because that's the meaning of a raw `Exception` and that's why `throws Exception` and `catch(Exception)` is generally a bad idea. The system works, it just might cause an inconvenience in this situation where idioms have not been used correctly in the first place. I don't think the situation is possible, though because `FooDiskItem.write` in 1.4.x can't throw Exception anymore.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org