gallandarakhneorg / afc

Arakhnê Foundation Classes
http://www.arakhne.org/afc/
Apache License 2.0
14 stars 8 forks source link

Fixing squid: S1166 Exception handlers should preserve the original exception part 1 #97

Closed devFozgul closed 8 years ago

devFozgul commented 8 years ago

This pull request is focused on resolving occurrences of Sonar rule squid:S1166- “ Exception handlers should preserve the original exception”. This PR will remove 120 min TD.
You can find more information about the issues here: https://dev.eclipse.org/sonar/rules/show/squid:S1166 Please let me know if you have any questions. Fevzi Ozgul

gallandarakhneorg commented 8 years ago

I'm not sure we need to log the exceptions that have been considered in this PR. Indeed, the exception are silently caught for being managed later in the code. Putting them on the logger have two drawbacks:

@devFozgul, @ngaud, what is your opinion?

devFozgul commented 8 years ago

Do we not loose information? We will not be knowing what has happened if an exception occurs. We could rethrow them instead of logging, perhaps?

On Tue, Jun 7, 2016 at 12:13 PM, Stéphane Galland notifications@github.com wrote:

I'm not sure we need to log the exceptions that have been considered in this PR. Indeed, the exception are silently caught for being managed later in the code. Putting them on the logger have two drawbacks:

  • It make the log bigger without useful information
  • The code is slower to be run.

@devFozgul https://github.com/devFozgul, @ngaud https://github.com/ngaud, what is your opinion?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/gallandarakhneorg/afc/pull/97#issuecomment-224224124, or mute the thread https://github.com/notifications/unsubscribe/AR09BHrJGySkJ4nC1FT_ns8qgH3BYx4Rks5qJTYigaJpZM4Ivmqc .

gallandarakhneorg commented 8 years ago

Information is not lost because the code is testing all the possibilities, and if no one is valid, the line Class.class.cast(obj); throws a ClassCastException that is clearly stated the problem of conversion. Logging the "internal" conversion error is simply too verbose, from my point of view.