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

Enhance validateIconSize method with comprehensive exception handling #9623

Conversation

ArvindReddySheelam
Copy link

Enhance validateIconSize method with comprehensive exception handling

  • Added handling for RuntimeException to catch unchecked exceptions and rethrow as SecurityException.
  • Added a catch block for general Exception to handle any other unexpected exceptions and rethrow as SecurityException.
  • Updated Javadoc to document that SecurityException will be thrown for invalid icon sizes or other errors during validation.
    This PR improves the robustness of the validateIconSize method by introducing comprehensive exception handling and updating the Javadoc documentation accordingly.

Key Enhancements:

1, RuntimeException Handling:

Added a try-catch block to handle RuntimeException, which includes unchecked exceptions such as NullPointerException or IllegalArgumentException.
If a RuntimeException occurs during the validation process, it is caught and rethrown as a SecurityException with an appropriate message. This ensures that the method fails gracefully and provides meaningful error information.

2, General Exception Handling:

Introduced an additional catch block for the broader Exception class. This catch block ensures that any other unexpected exceptions are also caught and rethrown as SecurityException.
This approach guarantees that any error, whether checked or unchecked, is captured and appropriately handled, improving the method's reliability.

3, Javadoc Update:

The Javadoc for the validateIconSize method has been updated to reflect the changes. It now clearly documents that a SecurityException will be thrown not only for invalid icon sizes but also for any other errors that might occur during validation.

@zbynek
Copy link
Contributor

zbynek commented Aug 20, 2024

The code is redundant (for code blocks that do not throw declared exceptions it's useless to handle RuntimeException and Exception separately) and incorrectly formatted (see Checks tab of this PR). Also it's highly questionable whether SecurityException "provides meaningful error information" compared to the wrapped runtime exception (which is very unlikely to be thrown in the first place).

If you want to salvage this PR, I'd suggest to revert the code changes and only add the JavaDoc, to fix the formatting problems try mvn spotless:apply. You could also add a @NonNull anotation to the iconSize argument.

In general it's more efficient if you look for for an existing newbie-friendly issue that you could solve instead of doing ad-hoc improvements like this.

@ArvindReddySheelam ArvindReddySheelam deleted the UncheckedExceptionHandling branch August 21, 2024 06:15
@ArvindReddySheelam
Copy link
Author

Hi! @zbynek thanks for the review i made another PR with the changes that you requested tq reviews from contributors like you helped me to how to behave and deal with the stuff so thanks again.

@zbynek
Copy link
Contributor

zbynek commented Aug 21, 2024

I made another PR

#9637

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

Successfully merging this pull request may close these issues.

2 participants