Issue with the below code
-
Hi, I was looking for some guidance on best practices regarding error handling.
public class DemoController : Controller
{
private readonly ILogger _logger;public DemoController(ILogger logger) { \_logger = logger; } public ActionResult Index() { try { //Do something... } catch (Exception ex) { \_logger.Error(ex.ToString()); } return View(); } }
I was therefore wondering if anyone would have any suggestions to improve this code? Thanks
The Dark Knight
-
Hi, I was looking for some guidance on best practices regarding error handling.
public class DemoController : Controller
{
private readonly ILogger _logger;public DemoController(ILogger logger) { \_logger = logger; } public ActionResult Index() { try { //Do something... } catch (Exception ex) { \_logger.Error(ex.ToString()); } return View(); } }
I was therefore wondering if anyone would have any suggestions to improve this code? Thanks
The Dark Knight
Ask the person who told you it was sub-standard what their issues are. They could be referring to the fact that catching just "Exception" is too generic, that you should aim for specific exceptions (that rarely happens in the real world though). Or it could be the fact that you just log the exception and do nothing with it, you go on to return the View as normal so the user has no real knowledge that the code failed.
-
Hi, I was looking for some guidance on best practices regarding error handling.
public class DemoController : Controller
{
private readonly ILogger _logger;public DemoController(ILogger logger) { \_logger = logger; } public ActionResult Index() { try { //Do something... } catch (Exception ex) { \_logger.Error(ex.ToString()); } return View(); } }
I was therefore wondering if anyone would have any suggestions to improve this code? Thanks
The Dark Knight
It's usually good practice to inform the user that an error occurred, and therefore the output might not be trustworthy or the input was not accepted by the system.
"There are three kinds of lies: lies, damned lies and statistics." - Benjamin Disraeli
-
Ask the person who told you it was sub-standard what their issues are. They could be referring to the fact that catching just "Exception" is too generic, that you should aim for specific exceptions (that rarely happens in the real world though). Or it could be the fact that you just log the exception and do nothing with it, you go on to return the View as normal so the user has no real knowledge that the code failed.
Thanks, makes more sense to me now.
Paul McGann
-
It's usually good practice to inform the user that an error occurred, and therefore the output might not be trustworthy or the input was not accepted by the system.
"There are three kinds of lies: lies, damned lies and statistics." - Benjamin Disraeli
Thanks Benjamin, i'll keep this in mind for future.
Paul McGann
-
Hi, I was looking for some guidance on best practices regarding error handling.
public class DemoController : Controller
{
private readonly ILogger _logger;public DemoController(ILogger logger) { \_logger = logger; } public ActionResult Index() { try { //Do something... } catch (Exception ex) { \_logger.Error(ex.ToString()); } return View(); } }
I was therefore wondering if anyone would have any suggestions to improve this code? Thanks
The Dark Knight
The "best wisdom" I have found re error-handling is in the writings of Eric Lippert, a key .NET architect: his "classic" piece often cited by other language gurus: [^]. Also: [^], [^]. A summary based on Eric's writings: [^]. There's a 3 part series on "Dr. Spock Engineer" blog I think is worth studying that starts here: [^]. I favor always throwing an Exception except in very special cases like within an enumeration of data where partial results are acceptable and usable.
«Differences between Big-Endians, who broke eggs at the larger end, and Little-Endians gave rise to six rebellions: one Emperor lost his life, another his crown. The Lilliputian religion says an egg should be broken on the convenient end, which is now interpreted by the Lilliputians as the smaller end. Big-Endians gained favor in Blefuscu.» J. Swift, 'Gulliver's Travels,' 1726CE
-
The "best wisdom" I have found re error-handling is in the writings of Eric Lippert, a key .NET architect: his "classic" piece often cited by other language gurus: [^]. Also: [^], [^]. A summary based on Eric's writings: [^]. There's a 3 part series on "Dr. Spock Engineer" blog I think is worth studying that starts here: [^]. I favor always throwing an Exception except in very special cases like within an enumeration of data where partial results are acceptable and usable.
«Differences between Big-Endians, who broke eggs at the larger end, and Little-Endians gave rise to six rebellions: one Emperor lost his life, another his crown. The Lilliputian religion says an egg should be broken on the convenient end, which is now interpreted by the Lilliputians as the smaller end. Big-Endians gained favor in Blefuscu.» J. Swift, 'Gulliver's Travels,' 1726CE
Thanks Bill, great articles and advice appreciate everyone's feedback.
Paul McGann
-
Hi, I was looking for some guidance on best practices regarding error handling.
public class DemoController : Controller
{
private readonly ILogger _logger;public DemoController(ILogger logger) { \_logger = logger; } public ActionResult Index() { try { //Do something... } catch (Exception ex) { \_logger.Error(ex.ToString()); } return View(); } }
I was therefore wondering if anyone would have any suggestions to improve this code? Thanks
The Dark Knight
Logging the exception is useful, but I would recommend also throwing (not rethrowing) it. What I mean is:
try {
DoSomething();
}
catch (Exception ex) {
this._logger.Log(ex);
throw;
}/ravi
My new year resolution: 2048 x 1536 Home | Articles | My .NET bits | Freeware ravib(at)ravib(dot)com
-
Hi, I was looking for some guidance on best practices regarding error handling.
public class DemoController : Controller
{
private readonly ILogger _logger;public DemoController(ILogger logger) { \_logger = logger; } public ActionResult Index() { try { //Do something... } catch (Exception ex) { \_logger.Error(ex.ToString()); } return View(); } }
I was therefore wondering if anyone would have any suggestions to improve this code? Thanks
The Dark Knight
Yes, instead of writing the same old boiler-code (as they call it), I would use PostSharp to create code-injection attributes which would automatically wrap the method with your try-catch{} exception-logger. This reduces the redundant code and improves the visibility of your class code.
Ben Scharbach Temporalwars.Com YouTube:Ben Scharbach