Does wrapping and rethrowing an exception impact performance?

Question

I have followed this way of handling exception in my application. But my lead said I am doing it wrong. I am simply wrapping and rethrowing the same exception, which will impact performance.

What is wrong with my approach? Does anyone have any suggestions on how I can log and handle the exception here?

public class BusinessRepository : IBusinessRepo
{
    public List<Employee> GetEmployees()
    {
        try
        {
            //do some DB operations
        }
        catch (SQLException sqlex)
        {
            Logger.Log("Exception detail with full stack trace");
            throw new DALException(sqlex, "Error in data access layer");
        }

    }
}
public class BusinessLayerClass : IBusinessLayer
{
    private readonly IBusinessRepo Repo;
    public BusinessLayerClass(IBusinessRepo rep)
    {
        Repo = rep;
    }
    public List<Employee> GetEmployees()
    {
        try
        {
           List<Employee> emps= return Repo.GetEmployees();
        }
        catch (DALException dex)
        {
            //do nothin as it got already logged
            throw;
        }
        catch (Exception ex)
        {
            Logger.Log(ex, "Business layer ex");
            throw new BusinessLayerEx(ex);
        }
    }
}

public class HomeController : Controller
{
    public ActionResult Index()
    {
        try
        {
            List < Employee >= BusinessLayerClass.GetEmployees();

        }
        catch (DALException)
        {
            //show error msg to user
        }
        catch (BusinessLayerEx)
        {
            //show error msg to user
        }
        catch (Exception ex)
        {
            Logger.Log();
            //show error msg to user
        }
        return View(emps);
    }
 }

Do i follow right way of bubbling and handling and logging shown above?

Answer 1

I'm inclined to agree with your way of doing this, as long as two conditions are met:

1. Your Logger.Log statements log something more meaningful/useful than what you've indicated here (I'm guessing your code here is just a sample message indicating the error is logged). If it provides information you can use to track down the cause of the exception, good.
2. Your //show error msg to user comments mean that in that location, you render a nice view explaining that an error has occured, and you aren't just showing a default exception screen/strack trace.

As far as your throw; when you catch the DALException you just threw: that's fine. Your goal here seems to be to catch any exception coming out of the previous layer and log it, throwing your own exception afterwards. Since DALException will only be thrown if you've already logged another error and thrown it yourself, it's perfectly fine to let it bubble up past this level.

Answer 2

The general rule of thumb for exceptions is do not catch them unless you can "do something about it", i.e. add value. Ideally this would be some kind of graceful recovery to the point that the user never knows there was a hiccup, but at the very minimum this would include logging the exception -- which you are doing.

Do not catch an exception only to immediately re-throw it. That adds no value. (An exception to this might be if you need to change the type of exception to something more informative/appropriate to the context).