0

I'm using sonarlint with C#. Something strange happens when I write two try..catch blocks that catch blocks do a same thing, like this:

try
{
    // Sth
}
catch (Exception ex)
{
    // Log sth else
    logger.LogError(ex.ToString());
}

try
{
    // Sth else
}
catch (Exception ex)
{
    // Log sth else
    logger.LogError(ex.ToString());
}

It suggests me to combine these two blocks in a single one, maybe like this

try
{
    // Sth
    // Sth else
}
catch (Exception ex)
{
    // Log sth else
    logger.LogError(ex.ToString());
}

but obviously this will not do the same thing, because the part // Sth else will not be executed if exception happens in // Sth part.

What is going on here? Am I making a mistake or it is on sonarlint?

Ahmad
  • 5,551
  • 8
  • 41
  • 57
  • 4
    You are correct, this is a failed optimization. – TheBatman Mar 11 '20 at 17:41
  • 1
    Sonarlint is giving you a false positive. –  Mar 11 '20 at 17:42
  • I think you should refactor your two process in different methos to avoid this issue, sonarlint don't knows why you need the two process must be executed, the linter thinks that this case must be simplified. – svladimirrc Mar 11 '20 at 18:08
  • @svladimirrc No, the linter is suggesting it *can* be simplified, not that it *must* be. A better solution would be to add the attribute/comment to the code to tell Sonar to ignore this particular rule for this particular method. –  Mar 11 '20 at 19:06
  • Usually writing `try..catch` it this way means that throwing exception is a part of your flow, which is a code smell. If you know that first part of your code can fail and you are expecting this, instead of throwing exception you should return some status code instead. Keep in mind that exceptions are expensive: https://stackoverflow.com/questions/891217/how-expensive-are-exceptions-in-c – Peska Mar 11 '20 at 19:11
  • @Ami MUST, SHOULD depending on the approach. If we are treating the warnings as errors or not. – svladimirrc Mar 11 '20 at 19:39
  • @svladimirrc In this case, it neither should, nor must, be fixed. It's a **false positive**, and the linter is offering a suggestion. It's not your boss. You are free to ignore it or tell it to ignore the rule here because you, the programmer, know better than the static analyzer, which is making an educated guess. –  Mar 11 '20 at 20:20
  • As I said, linter don´t know what the programmer needs, but, the linter says, hey here there are two blocks of code that can be simplified, thats indicate that or you need to refactor your code or you can simplify it, From my point of view the two blocks needs to be separated like a good practice and separate responsabilities. If you dont want to see warnings of linter then you can supress the case, but I think is not the point. – svladimirrc Mar 11 '20 at 21:43

0 Answers0