Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify API #309

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Simplify API #309

wants to merge 1 commit into from

Conversation

basil
Copy link

@basil basil commented May 10, 2024

Not sure why this API has calling conventions that vary so drastically from method to method. Since this is still in M2 stage, it seems like it is still not too late to fix this. One problem with the existing code is that it violates Effective Java 3rd Edition §74, which says:

Use the Javadoc @throws tag to document each exception that a method can throw, but do not use the throws keyword on unchecked exceptions.

Another problem with the existing code is that FileItem#getString(Charset) is inconsistent with FileItem#getString()—one declares throws IOException while the other does not, yet it is not obvious how the presence of a Charset argument should cause any difference in calling convention.

Yet another problem with the existing code is that DiskFileItem#getString(java.nio.charset.Charset) claims to throw IOException but is actually implemented with DiskFileItem#get() which actually throws UncheckedIOException.

This PR fixes all of these problems by eliminating any usages of UncheckedIOException in favor of simple usages of throws IOException.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant