1
\$\begingroup\$

I am trying to write an exception handler/parser, which I want to return from DAL to the business layer. I am writing below class because I don't want my business layer to know if I am using EF under my DAL.

Please suggest if below code adhere to SOLID principles?

using System;
using System.Collections.Generic;
using System.Data.Entity;
using System.Data.Entity.Infrastructure;
using System.Data.Entity.Validation;

namespace WebAPis
{
    public enum ExceptionType
    {
        Unknown,
        EntityModified,
        EntityValidation,
        EntityUpdateException

    }
    public class EfResponse
    {
        private Exception Exception { get; set; }
        public void ParseException(Exception ex)
        {
            this.Exception = ex;
        }
        public bool IsSuccess { get { return this.Exception == null; } }
        public int RowsModified { get; set; }
        public ExceptionType Type
        {
            get
            {
                var type = ExceptionType.Unknown;

                if (this.Exception is DbEntityValidationException)
                    return ExceptionType.EntityValidation;

                if (this.Exception is DbUpdateException)
                    return ExceptionType.EntityUpdateException;

                if (this.Exception is DbUpdateConcurrencyException)
                    return ExceptionType.EntityModified;
                // add more types here
                return type;
            }
        }

        public DbUpdateExceptionDetail GetDbUpdateExceptionDetail
        {
            get
            {
                if (this.Type != ExceptionType.EntityUpdateException)
                    throw new Exception("Invaid exception details invoked");

                var toReturn = new DbUpdateExceptionDetail()
                {
                    // Parse exception here
                    ColumnName = "test column name"
                };
                return toReturn;

            }
        }
        public ConcurrencyExceptionDetail GetConcurrencyExceptionDetail
        {
            get
            {
                if (this.Type != ExceptionType.EntityModified)
                    throw new Exception("Invaid exception details invoked");

                var toReturn = new ConcurrencyExceptionDetail()
                {
                    // Parse exception here
                    MyProperty = "test property name"
                };
                return toReturn;
            }
        }
    }

    // Exception models
    public abstract class ExceptionDetail
    {
        public List<string> Exceptions { get; set; }
    }
    public class DbUpdateExceptionDetail : ExceptionDetail
    {
        public string ColumnName { get; set; }
    }

    public class ConcurrencyExceptionDetail : ExceptionDetail
    {
        public string MyProperty { get; set; }
    }

    // example dbcontext
    public class MyDbcontext : DbContext
    {
        public EfResponse Save()
        {
            var toReturn = new EfResponse();
            try
            {
                toReturn.RowsModified = this.SaveChanges();
            }
            catch (Exception ex)
            {
                toReturn.ParseException(ex);
            }

            return toReturn;
        }
    }
}
\$\endgroup\$
  • 3
    \$\begingroup\$ You are super over-engineering your app. I am trying to write an exception handler/parser - this is such a bad idea. I don't want my business layer to know if I am using EF under my DAL. then use a repository. Besides it's pretty pretty PRETTY unlikely that you'll ever exchange EF for anything else. \$\endgroup\$ – t3chb0t May 10 '17 at 4:13
  • \$\begingroup\$ @t3ch0t the requirement of my solution is to notify users that why I am rejecting api call. If request encountered an unique constraint or concurrency exception, I want to notify user which particular value in the entity has failed. A quick sol is to not to handle concurrency or dbupdate exception at repository level but catch it my web api base controller but problem is my business layer will be consumed by some other non ui components. This is the reason I want to tell my Business layer why exception occured. I hope my requirement justify why I want to handle it and tell my business layer. \$\endgroup\$ – jNet May 10 '17 at 22:49
  • \$\begingroup\$ Additionally, above class is not leaking EF information into business layer. This is the reason this class is parsing exception and passing my custom information back to business layer. \$\endgroup\$ – jNet May 11 '17 at 0:45
2
\$\begingroup\$

I think this is not a good idea for the following few reasons.

You write

I don't want my business layer to know if I am using EF under my DAL.

  • and yet the ExecptionType enum contains all the values strictly related to Entity Framework.

  • Also the name of the EfResponse object already suggests it's Entity Framework related too with its Ef prefix.

  • Then the public class MyDbcontext : DbContext is derived from DbContext which agian is Entity Framework.

This means that the EfResponse and the MyDbcontext aren't abstract enough and they don't separate you from either Entity Framework or any other DAL. They just use different names for some EF types.


Other things that aren't implemented correctly are the two detail properties. We don't use the Get prefix for them. Only for methods. The way they work is also a little bit strange. I first need to check the type of the exception to know which property I'm allowed to use or otherwise it'll might throw. They should return null if there is no exception of this type. The only exception they are allowed to throw is the InvalidOperationException if a EfResponse object is in the Success state.

The property RowsModified should not be publically settable. This should be a part of the constructor.


Some time ago there was a question about something similar that you might find interesting: Refactoring away from exceptions. I think what you are trying to do is virtually the same. I tried it and it's a real mess when you once start using it. You're better off with a simple repository encapsulating all db operations an their exceptions.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Please ignore the namings of my classes as these are just the blueprints. MyDbContext is written to just give example. I appriciate your suggestion about: property name should not be prefixed Get. RowsModified can't have public setter. \$\endgroup\$ – jNet May 11 '17 at 8:16
  • \$\begingroup\$ Details properties should return null. I am not convinced about it because I am throwing exception such that consumer should know if he is using wrong property to get details. What are your comments? \$\endgroup\$ – jNet May 11 '17 at 8:17
  • \$\begingroup\$ @jNet my comments are already in the review. The consumer should not even be trying to get those properties if the state of the object is not valid for getting them, this is, there is no error. So either there is an error detail for a type or not or the state does not allow to get it. \$\endgroup\$ – t3chb0t May 11 '17 at 8:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.