• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Paul Clapham
  • Tim Cooke
  • Jeanne Boyarsky
  • Liutauras Vilda
Sheriffs:
  • Frank Carver
  • Henry Wong
  • Ron McLeod
Saloon Keepers:
  • Tim Moores
  • Frits Walraven
  • Tim Holloway
  • Stephan van Hulst
  • Carey Brown
Bartenders:
  • Al Hobbs
  • Piet Souris
  • Himai Minh

SONAR issue - Avoid re-throwing exception

 
Greenhorn
Posts: 5
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hello,

I have a sonar violation and I would like to how to fix this. It complains "Avoid re-throwing exception". Thanks for your time

 
Master Rancher
Posts: 4271
57
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I recommend disabling that inspection because it's simply wrong here.
 
kark Kumr
Greenhorn
Posts: 5
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hello Mike,

Thanks for the recommendation but is there a way we can modify the code. wrapping in another exception like below

 
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

kark Kumr wrote:Hello Mike,

Thanks for the recommendation but is there a way we can modify the code. wrapping in another exception like below



You're already doing that:



If you want to wrap it in your own custom Exception class instead of ReportException, you just need to make sure that class has a constructor that takes a String and calls a super(...) c'tor passing that string to it.

Your current code handles ReportException specially, just re-throwing it, and wrapping any other Exception in a ReportException.

 
Mike Simmons
Master Rancher
Posts: 4271
57
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
[I'm responding to kark Kumr, not Jeff - I saw his post only afterwards]

I'm sure you could, but that seems to defeat the purpose of the original code - which seems to be to wrap everything as a ReportException unless it's already a ReportException. There's no need to introduce another layer of wrapping on something that's already a ReportException. And even if you did this, you'll still have the same sort of problem one level up, in code that calls this code. What if you need to catch other exceptions and convert them to NewWrappedReportException, unless they're already NewWrappedReportException? Should you then introduce EvenNewerWrappedReportException to wrap those? No, I think the original code is much cleaner. Tell Sonar/FindBugs/whatever to take a hike here - it's wrong.
 
Jeff Verdegan
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Mike Simmons wrote:Tell Sonar/FindBugz/whatever to take a hike here - it's wrong.



The idea is correct in general, but it would be nice if the inspection could detect cases like this, where we're only rethrowing a specific exception as a way to treat it separately from a broader family of exceptions that would otherwise include this one.
 
Mike Simmons
Master Rancher
Posts: 4271
57
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Yeah, it seems like anytime the catch-and-rethrow is followed by another catch block for a supertype of the current exception, this inspection should accept that as valid code and shut up. Doesn't seem like a hard thing to check for.

I find that when people use these checking tools and turn on all available checks, you get a lot of tedious false warnings. Some checks just aren't that accurate. Code checking tools need to be tuned, using only the checks that are usually useful. Because if they give false warnings too often, people just get in the habit of ignoring them - which defeats the purpose.
 
Jeff Verdegan
Bartender
Posts: 6109
6
Android IntelliJ IDE Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Mike Simmons wrote:
I find that when people use these checking tools and turn on all available checks, you get a lot of tedious false warnings. Some checks just aren't that accurate. Code checking tools need to be tuned, using only the checks that are usually useful. Because if they give false warnings too often, people just get in the habit of ignoring them - which defeats the purpose.



Yeah, I'm constantly tweaking my IntelliJ Inspection settings. It's great that it can accommodate pretty much any combination of standards you can think of, but when, for instance, you come into an existing codebase written by a team that didn't have rigid standards, you'll get files full of yellow bars and you have to either turn off inspections you might otherwise have found useful; modify code that you wouldn't otherwise modify, just for the sake of getting rid of warnings; or live with possibly missing some legitimate warnings as they get lost in the clutter.

Ahh, the torturous life of a conscientious developer!
 
kark Kumr
Greenhorn
Posts: 5
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Alright. Thanks every one for their valuable suggestions.
 
Mike Simmons
Master Rancher
Posts: 4271
57
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thinking on it some more, I suppose you could do something like this:

And then your client code looks like this:

Normally it's discouraged to use instanceof when there are alternate methods available, like type-specific catch blocks. But what I like about this solution is that it allows the client code to be clean and simple - moreso than the original solution. The minor ickiness of using instanceof is hidden away inside the ReportException class, where most people are unlikely to see it or care about it. It also avoids the Sonar warning, but that is accidental. Sonar is still wrong here.

The "message" parameter is a bit problematic, as it's something only used if the exception is not already a ReportException. That's not necessarily obvious to casual viewers of the code, so it probably needs better documentation.
 
Bartender
Posts: 1166
17
Netbeans IDE Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Mike Simmons wrote:But what I like about this solution



What I hate about this solution is that you are writing code to deal with a flaw in the IDE ! The IDE is very wrong in this case and it should be reported as a bug and for the moment that particular checking should be turned off.
 
Mike Simmons
Master Rancher
Posts: 4271
57
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
No, not the IDE. Sonar or maybe FindBugs.

But while I was thinking about it because of the Sonar warning and this post, the reason I like it is independent of that. I dislike appeasing a broken tool. But I like anything that makes a class easier for clients to use. I think this qualifies, and that's why I posted it.
 
Mike Simmons
Master Rancher
Posts: 4271
57
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
After a little research, I think it's probably a PMD warning, called from Sonar. So forget I mentioned FindBugs (spelling corrected now), and it's not really fair to blame Sonar either. Maybe not even PMD in general. It's just this one particular check which is broken. It's only Sonar's fault if they made a choice to include this check as one of their default options.
 
Richard Tookey
Bartender
Posts: 1166
17
Netbeans IDE Java Linux
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Mike Simmons wrote:No, not the IDE. Sonar or maybe FindBugs.



OK not the IDE therefore in my previous response just replace IDE with the name of whatever tool is creating the flawed warning message.
 
My name is Inigo Montoya, you killed my father, prepare to read a tiny ad:
the value of filler advertising in 2021
https://coderanch.com/t/730886/filler-advertising
reply
    Bookmark Topic Watch Topic
  • New Topic