none
N-Layer Application Design. RRS feed

  • Question

  • Hello Everyone,

    Right now I am getting started on an N-Layer Web Application.

    I have some questions about proper error handling techniques within my DAL, and the Base class within the DAL.

    Here is my idea.

    I have a SQLBase class in the DAL as we only use SQL Server, and only execute stored procedures.

    Then, I have a bunch of classes in the DAL that inherit from the SQLBase class.

    My idea was that the SQL Base class should NOT catch any exceptions, and it should let the client do that.

    Or, the alternative would be to catch the errors in the SQL Base class, log them, and rethrow the exception so it could be caught, wrapped in a new custom application exception and thrown to the ui..

    Am I completely off here?


    Here is the code that I have started with. 

    Imports System.Data.SqlClient
    Imports System.Runtime.Serialization
    
    Public MustInherit Class SQLBase
    
        Private Function CreateConnection() As SqlConnection
            Using conn As New SqlConnection
                conn.ConnectionString = "read from application settings"
                Return conn
            End Using
        End Function
    
        Private Sub OpenConnection(ByVal conn As SqlConnection)
            Try
                conn.Open()
            Catch ex As SqlException
                Throw New CSException("A connection could not be made to the database.", _
                                      "APPNAME is unable to connect to its data source at this time.", _
                                      "Nothing has been affected.", _
                                      "Try again", _
                                      "support@whatever.com", _
                                      ex)
    
            End Try
        End Sub
    
        Protected Function ExecuteNonQuery(ByVal sprocName As String, ByVal ParamArray sprocParams As SqlParameter()) As Integer
            Dim retval As Integer
            Using cmd As New SqlCommand
                Try
                    With cmd
                        .CommandType = CommandType.StoredProcedure
                        .CommandText = sprocName
                        .Connection = CreateConnection()
    
                        If sprocParams IsNot Nothing Then
                            For index As Integer = 0 To sprocParams.Length - 1
                                .Parameters.Add(sprocParams(index))
                            Next
                        End If
                    End With
    
                    OpenConnection(cmd.Connection)
    
                    retval = cmd.ExecuteNonQuery()
                Finally
                    'clean up
                    cmd.Connection.Close()
                End Try
            End Using
            Return retval
        End Function
    End Class
    
    
    Public Class AccountRepository : Inherits SQLBase
    
        Public Function ChangePassword(ByVal ID As Integer, ByVal Password As String) As Boolean
            Dim retval As Integer
            Dim result As Boolean = False
    
            Try
                retval = ExecuteNonQuery("ChangePassword", _
                                         New SqlParameter("@ID", ID), _
                                         New SqlParameter("@Password", Password))
    
                If retval > 0 Then result = True
    
            Catch sqlex As SqlException
                Throw New CSException("Unable to change password.", _
                                      "An error happened while changing your password.", _
                                      "Your password has not be changed.", _
                                      "If problem presists call tech support", _
                                      "Call tech support", _
                                      sqlex)
            Catch ex As Exception
                'throw new csexception.
    
            End Try
            Return result
        End Function
    
    End Class
    
    
    'Custom Application Exception
    Public Class CSException : Inherits ApplicationException
    
    #Region "Private Attributes"
        Private m_WhatHappened As String
        Private m_WhatsBeenAffected As String
        Private m_WhatCanUserDo As String
        Private m_SupportInfo As String
    #End Region
    
    #Region "Public Attributes"
        Public ReadOnly Property WhatHappened() As String
            Get
                Return m_WhatHappened
            End Get
        End Property
    
        Public ReadOnly Property WhatsAffected() As String
            Get
                Return m_WhatsBeenAffected
            End Get
        End Property
    
        Public ReadOnly Property WhatCanUserDo() As String
            Get
                Return m_WhatCanUserDo
            End Get
        End Property
    
        Public ReadOnly Property SupportInfo() As String
            Get
                Return m_SupportInfo
            End Get
        End Property
    #End Region
    
    #Region "Constructors"
    
        Public Sub New()
    
        End Sub
    
        Public Sub New(ByVal Message As String)
            MyBase.New(Message)
        End Sub
    
        Public Sub New(ByVal Message As String, ByVal WhatHappened As String, ByVal WhatsAffected As String, ByVal WhatCanUserDo As String, ByVal SupportInfo As String, ByVal InnerException As Exception)
            MyBase.New(Message, InnerException)
            m_WhatHappened = WhatHappened
            m_WhatsBeenAffected = WhatsAffected
            m_WhatCanUserDo = WhatCanUserDo
            m_SupportInfo = SupportInfo
        End Sub
    
        Protected Sub New(ByVal info As SerializationInfo, ByVal context As StreamingContext)
            MyBase.New(info, context)
            m_WhatHappened = info.GetString("WhatHappened")
            m_WhatsBeenAffected = info.GetString("WhatsBeenAffected")
            m_WhatCanUserDo = info.GetString("WhatCanUserDo")
            m_SupportInfo = info.GetString("SupportInfo")
        End Sub
    
    #End Region
    
        Public Overloads Overrides Sub GetObjectData(ByVal info As SerializationInfo, ByVal context As StreamingContext)
            info.AddValue("WhatsBeenAffected", m_WhatsBeenAffected, GetType([String]))
            info.AddValue("WhatCanUserDo", m_WhatCanUserDo, GetType([String]))
            info.AddValue("SupportInfo", m_SupportInfo, GetType([String]))
            info.AddValue("WhatHappened", m_WhatHappened, GetType([String]))
            MyBase.GetObjectData(info, context)
        End Sub
    
    End Class

    Am I moving in the right direction with this or am I way off?


    Is the error handling technique in the above code correct?

    If not, what changes should be made?


    Any help would be greatly appreciated...

    Thank you.

     

    • Edited by intertek Wednesday, March 17, 2010 12:46 AM
    Tuesday, March 16, 2010 6:18 PM

Answers



  • 1. However, with your approach, I would have to create a custom exception for each domain object.

    Not necessary but partially true. I generally create one exception per data service (IEmpDataService) class and sometime data service class might be dealing with only one domain object and sometimes more then one. Consider the case-

           class Invoice
            {
                public List<Item> Items { get; set; }
            }
            class Item
            {
    
            }
    
            interface IInvoiceDataService
            {
    
                void Add(Invoice invoice);
            }
    
           class InvoiceException:Exception
           {
              //..............
    
           }
    in above case I'll not create a seperate exception class for "Item" domain object.


    2. Another thing I would also like to point out is that, with your code is that your letting some SQL Exceptions bubble up and out of the DAL, which to me is bad practice, as it could allow sensitive information to be leaked out of the DAL such as Stored Proc names, ect.

    You can always have variations according to your requirment but here are some points-
    a. Generally datalayer is not directly used by client (but I'm not sure about architecture). It is business layer, a seperate DLL at same machine, which directly use datalayer and there is not harm if the details come to that DLL. It is at the point when your error message(or exception, fault) conveyed to outer clients, you would wish to suppress that information and return a generic kind of exception instead after logging.

    b. Even if you wish to return custom exception instead of SQLException, that custome exception is going to have that SQLException as its innertException (pretty much what you have done) so such details are already our of datalayer :). In fact this actual exception is of paramount importance for debugging so it must be logged. I think having logging mechanism at top level (at one place) is much clean than
    scattered at all over the places like data layers (of course logging is used in whole application for debuging purpose).

    Thanks,
    Gurmit 


    • Marked as answer by intertek Thursday, March 18, 2010 7:04 AM
    Thursday, March 18, 2010 6:06 AM

All replies

  • I will suggest rather using your own code, try to use entity framework or any other Object relation mapping tool. This will give to builtin functionality to perform database operations without writing any code. You can then build your business layer by interacting with your business objects/entities
    Tuesday, March 16, 2010 9:06 PM
  • Thanks, but the question was about error handling, not about what technology to use. 

    There is absolutely nothing wrong with writing your own data access code.

    Wednesday, March 17, 2010 12:39 AM
  • Oh, I am sorry about the misunderstanding...

    My suggestion will be to define your own exception classes in DAL which you want to throw to business layer. Like if you get an SqlException at DAL rethrow your own DBException to BAL. In your exception try to construct proper message to clearly notify the reason of the error. This way your BAL doesnt have to deal with database/DAL specific exceptions and there will be abstraction even on the exception handling between the two layer.

    So tomorrow if you need to move to Oracle database, your BAL will be still unaffected.

    The same concept applies across multiple layers.

    Wednesday, March 17, 2010 6:52 AM

  • Hello intertek,

    Here are my suggestions. I have used them in my projects and found them quite useful-

    1. No need to do the logging the datalayer, log the exceptions at top level. Because you're not going to suppress the exception in datalayer; it will either popped up as it is or as a custom domain exception. You're always going to get the chance to log them at top level.

    2. Convert only to domain exception, when it make sense in your application. Let me help explain it using this example-

    enum EmpError
        {
            EmpAlreadyExist,
    
            NoEmpExist
        }
        class EmpException : Exception
        {
            public EmpError Reason { get; private set; }
    
            public EmpException(EmpError reason,String msg,Exception innerException)
                :base(msg,innerException)
            {
                Reason = reason;
            }
    
        }
    
        class Emp
        {
    
        }
        interface IEmpDataServce
        {
            void Add(Emp e);
        }
        class EmpDataService : IEmpDataServce
        {
    
            #region IEmpDataServce Members
    
            public void Add(Emp e)
            {
                try
                {
                    //execute the procedure to insert employee
                }
                catch (SqlException ex)
                {
                    if (ex.ErrorCode == 2627 (eg))
                    {
                        throw new EmpException(EmpError.EmpAlreadyExist, "Employee already exist", ex);
                    }
                    //otherwise let it popup, you'll not gain anything by converting to domain exception and having 
    //a generic error message, inside that. You can alway shows generic message (or whatever the action you
    // want to take)at top level with such unexpected exceptions.
    throw; } } #endregion }
    3. As visible in point 2; you can define your domain exception per data service class and have a reason field in it to identity the cause (you're intersted in). It much easier to take decision based on reason field than on string messages inside exception class. You'll only define enums in reason (e.g. EmpError) only for known domain error. I don't see any reason for having a unknown error field.

    4. To me your CSException class looks pretty complicated unless I'm missing your requirment. I'd prefer to keep it simple; similar to EmpException (please note it is not complete implementation) explained in step 2.

    5. As I have already explained the reasons in step 2, I'd not advise to throw CSException on any unknown SQLException in "ChangePasswordMethod".


    Thanks,
    Gurmit



    Wednesday, March 17, 2010 7:28 AM
  • @Muhd-Ahsan:  Thank you for your suggestion. 

    So I would create a set of exceptions in the DAL, and in the BLL?

    Doesn't this violate OOP practices?  Shouldn't I create common library between all layers, and put the Exceptions in that?

    Forgive me if I am way off, im still learning! ;)


    @Gurmit: Thank you.

    The CSException I agree, the CSException is a bit overkill.

    However, with your approach, I would have to create a custom exception for each domain object.

    Another thing I would also like to point out is that, with your code is that your letting some SQL Exceptions bubble up and out of the DAL, which to me is bad practice, as it could allow sensitive information to be leaked out of the DAL such as Stored Proc names, ect.

    This is why I was catching ALL sql exceptions and rethrowing them with information that is less sensitive.

    What is your take on this?



    Again, Thank you both for your replies.




     

    Wednesday, March 17, 2010 5:49 PM
  • Hi,

    I would suggest that you raise the error till the presentation layer and then log the same. You also have to ensure that the exception raised from the DAL layer and propogated back from BL and SL should have the entire call trace, which means you need to add custom messages so that when logged from the presenation layer the entire call can be traced.

    You can also do the logging before the PL as well which can be the Service Layer. Keep one place where all the exceptions are logged.

    Again you have to extra careful when your application is deployed in multi tiers, here you might have issues on how the raise the error from one tier to another.

    Hope this helps

    Thanks and Regards
    Azhar

    Mark as answer if this helps
    Thanks and Regards Azhar Amir
    Thursday, March 18, 2010 3:43 AM


  • 1. However, with your approach, I would have to create a custom exception for each domain object.

    Not necessary but partially true. I generally create one exception per data service (IEmpDataService) class and sometime data service class might be dealing with only one domain object and sometimes more then one. Consider the case-

           class Invoice
            {
                public List<Item> Items { get; set; }
            }
            class Item
            {
    
            }
    
            interface IInvoiceDataService
            {
    
                void Add(Invoice invoice);
            }
    
           class InvoiceException:Exception
           {
              //..............
    
           }
    in above case I'll not create a seperate exception class for "Item" domain object.


    2. Another thing I would also like to point out is that, with your code is that your letting some SQL Exceptions bubble up and out of the DAL, which to me is bad practice, as it could allow sensitive information to be leaked out of the DAL such as Stored Proc names, ect.

    You can always have variations according to your requirment but here are some points-
    a. Generally datalayer is not directly used by client (but I'm not sure about architecture). It is business layer, a seperate DLL at same machine, which directly use datalayer and there is not harm if the details come to that DLL. It is at the point when your error message(or exception, fault) conveyed to outer clients, you would wish to suppress that information and return a generic kind of exception instead after logging.

    b. Even if you wish to return custom exception instead of SQLException, that custome exception is going to have that SQLException as its innertException (pretty much what you have done) so such details are already our of datalayer :). In fact this actual exception is of paramount importance for debugging so it must be logged. I think having logging mechanism at top level (at one place) is much clean than
    scattered at all over the places like data layers (of course logging is used in whole application for debuging purpose).

    Thanks,
    Gurmit 


    • Marked as answer by intertek Thursday, March 18, 2010 7:04 AM
    Thursday, March 18, 2010 6:06 AM
  • Gurmit,

    I get what your saying now and I completely agree with you.

    Thank you for helping me out with this I really appreciate it!

    I can move forward with my project now. ;)




    Thursday, March 18, 2010 7:03 AM