Implementing repository pattern and DAL with stored procedures

Question

Using .net 3.5, implementation of the repository pattern along with enterprise library 5 and stored procedures. Did not use EF, stored procedures already available and VS2008 is limited to EF 3/4.

Looking at the following code, what are your recommendations for each section:

Domain/Repositories/IRepository.cs

public interface IRepository<T>
    {
        IEnumerable<T> Get();
        T Get(int Id);
        void Save(T model);
        void Delete(int Id);
    }

Domain/Repositories/RequestRepository.cs

(Similar code repeated for ResponseRepository, ApprovalRepository, etc...)

 public  class RequestRepository : IRepository<Request>
    {
        public IEnumerable<Request> Get()
        {
            return DataAcessWrapper.GetRequests();
        }

        public Request Get(int Id)
        {
            return DataAcessWrapper.GetRequest(Id);
        }

        public void Save(Request model)
        {
            DataAcessWrapper.SaveRequest(model);
        }

        public void Delete(int Id)
        {
            DataAcessWrapper.DeleteRequest(Id);
        }
    }

Domain/DataAccess/DataAccessWrapper.cs

public static class DataAcessWrapper
    {

        public static string _connectionString = ConfigurationManager.ConnectionStrings["ConnectionString"].ConnectionString;

        public static IEnumerable<Request> GetRequests(){
            SqlDatabase db = new SqlDatabase(_connectionString);
            var requests = db.ExecuteSprocAccessor<Request>("ResponseTracking_Get_Requests");
            return requests;
        }

        public static Request GetRequest(int requestId)
        {
            SqlDatabase db = new SqlDatabase(_connectionString);
            var request = db.ExecuteSprocAccessor<Request>("ResponseTracking_Get_Requests", requestId).ToList().FirstOrDefault();
            return request;
        }

        public static void SaveRequest(Request request)
        {
            SqlDatabase db = new SqlDatabase(_connectionString);
            DbCommand dbc = db.GetStoredProcCommand("ResponseTracking_Put_Request");
            db.AddInParameter(dbc, "@RequestID", DbType.Int32, request.RequestID);
            db.AddInParameter(dbc, "@Category", DbType.String, request.Category);
            db.AddInParameter(dbc, "@Title", DbType.String, request.Title);
            db.AddInParameter(dbc, "@DateReceived", DbType.DateTime2, request.DateReceived);
            db.AddInParameter(dbc, "@Source", DbType.String, request.Source);
            db.AddInParameter(dbc, "@ResponseRequired", DbType.String, request.ResponseRequired);
            db.AddInParameter(dbc, "@ResponseDateDue", DbType.DateTime2, request.ResponseDateDue);
            db.AddInParameter(dbc, "@ApprovalLevelRequired", DbType.String, request.ApprovalLevelRequired);
            db.AddInParameter(dbc, "@FollowUpDate", DbType.DateTime2, request.FollowUpDate);
            db.AddInParameter(dbc, "@Request", DbType.String, request.RequestDescription);
            db.AddInParameter(dbc, "@Status", DbType.String, request.Status);
            db.AddInParameter(dbc, "@Notes", DbType.String, request.Status);
            db.AddInParameter(dbc, "@ClosedDate", DbType.DateTime2, request.ClosedDate);
            db.AddInParameter(dbc, "@EmployeeID", DbType.String, request.EmployeeID);
            db.AddInParameter(dbc, "@CreationDateTime", DbType.DateTime2, request.CreationDateTime);
            db.ExecuteNonQuery(dbc);
        }

        public static void DeleteRequest(int requestId)
        {
            SqlDatabase db = new SqlDatabase(_connectionString);
            DbCommand dbc = db.GetStoredProcCommand("ResponseTracking_Delete_Request", requestId);
            db.ExecuteNonQuery(dbc);
        }
    }

Domain/Models/Request.cs

public class Request 
    {
        public int RequestID { get; set; } 
        public string Category { get; set; }  
        public string Title { get; set; }  
        public DateTime DateReceived { get; set;} 
        public string Source { get; set;}
        public string ResponseRequired{ get; set;}
        public DateTime ResponseDateDue { get; set;}
        public string ApprovalLevelRequired { get; set;} 
        public DateTime FollowUpDate {get; set;}
        public string RequestDescription {get; set;}
        public string Status {get; set;} 
        public string Notes { get; set; }
        public DateTime ClosedDate {get; set;}
        public string EmployeeID {get; set;} 
        public DateTime CreationDateTime {get; set;}  
    }

Answer 1

Did not use EF, stored procedures already available and VS2008 is limited to EF 3/4.

A quick Google search for "SqlDatabase class" yields an outdated MSDN page about an obscure class in an obscure Microsoft.Practices.EnterpriseLibrary.Data.Sql namespace:

Retired Content

This content is outdated and is no longer being maintained. It is provided as a courtesy for individuals who are still using these technologies. This page may contain URLs that were valid when originally published, but now link to sites or pages that no longer exist.

I'm somewhat hoping that's not the SqlDatabase class you're using. Why not just use ADO.NET?

I would create an ADO.NET .dbml model, where I'd reference all the stored procedures that need to be called by the application. I don't know if that could be called a "repository", but I'd implement it something like this:

(the using blocks ensure proper disposal of things that implement IDisposable)

public class RequestSqlDataService
{
    private readonly string _connectionString;

    public RequestSqlDataService(string connectionString)
    {
        _connectionString = connectionString;
    }

    public IDbConnection CreateConnection()
    {
        return new SqlConnection(_connectionString);
    }

    public IEnumerable<Request> Get()
    {
        using (var connection = CreateConnection())
        {
            return Get(connection);
        }
    }

    public IEnumerable<Request> Get(IDbConnection connection)
    {
        using (var context = new MyDataContext(connection))
        {
            return context.SelectRequests(null);
        }
    }

    public Request Get(int id)
    {
        using (var connection = CreateConnection())
        {
            return Get(connection, id);
        }
    }

    /* 
       an overload that takes an IDbConnection 
       allows wrapping these calls into a transaction! 
    */

    public Request Get(IConnection connection, int id)
    {
        using (var context = new MyDataContext(connection))
        {
            return context.SelectRequests(id);
        }
    }

    // rest of CRUD
}

In your .dbml you drag-and-drop the stored procedures from the server explorer into the designer, and then you give it a name that's code-friendly, like SelectRequests (the source can still be called ResponseTracking_Get_Requests, but that's less code-friendly), and you tell it that its return type is a Request object (which you can also create in the .dbml designer), or leave it an (auto-generated type) and work with an auto-generated ResponseTracking_Get_RequestsResult type instead.

---

Entity Framework also has support for stored procedures, and your Request class would make a perfectly suitable complex type (only missing a [ComplexType] attribute), and if the returned column names exactly match the property names, the mapping is automagic, but you have more control over how things are called than with ADO.NET.

---

One obvious advantage of using either an ADO.NET .dbml, or Entity Framework over what you have now, is that you just pass the parameter values as you would when you call any other method: the stored procedures become methods of the data context!

---

I have a couple of issues with your implementation:

• DataAccessWrapper looks like it's going to grow to know about every single stored procedure that the code needs to call.
• "repository" classes only make a conveniently focused wrapper on the wrapper.
• Everything being static in DataAccessWrapper is a bit scary: it means anyone could use any of these methods anywhere in the code, and no one would ever know.

What I mean with this last point is that despite IRepository<T>, the wrapper class could easily be used outside of the "repository" class, which would result in this:

public class SomeClass
{
    public void DoSomething()
    {
        //...
        var requests = DataAccessWrapper.GetRequests();
        //...
    }
}

...unless DataAccessWrapper is an internal class in a data-dedicated assembly, I think it would be better to implement that code inside your "repositories" - otherwise these repos are nothing but non-static wrappers over a static wrapper... which doesn't need to be static in the first place.

I'd prefer to have no choice but to do this:

public class SomeClass
{
    private readonly IRepository<Request> _service;

    public SomeClass(IRepository<Request> service)
    {
        _service = service;
    }

..and have the possibility of doing that:

    public void DoSomething()
    {
        //...

        using (var connection = _service.CreateConnection())
        using (var transaction = connection.BeginTransaction())
        {
            try
            {
                var requests = _service.GetRequests();
                //...
                transaction.Commit(); 
            }
            catch
            {
                transaction.Rollback();
                throw;
            }
        }

        //...
    }
}

I'd get rid of the static class and move that code into the repository implementations. Then I'd look into using EF or a Linq-to-SQL .dbml to replace the apparently outdated (never used it) Microsoft.Practices.EnterpriseLibrary.Data.Sql.

Answer 2

Both of these return a variable that you define within the block itself, so you should just return what you need instead of creating a variable

    public static IEnumerable<Request> GetRequests(){
        SqlDatabase db = new SqlDatabase(_connectionString);
        var requests = db.ExecuteSprocAccessor<Request>("ResponseTracking_Get_Requests");
        return requests;
    }

    public static Request GetRequest(int requestId)
    {
        SqlDatabase db = new SqlDatabase(_connectionString);
        var request = db.ExecuteSprocAccessor<Request>("ResponseTracking_Get_Requests", requestId).ToList().FirstOrDefault();
        return request;
    }

would be this instead

    public static IEnumerable<Request> GetRequests(){
        SqlDatabase db = new SqlDatabase(_connectionString);
        return db.ExecuteSprocAccessor<Request>("ResponseTracking_Get_Requests");
    }

    public static Request GetRequest(int requestId)
    {
        SqlDatabase db = new SqlDatabase(_connectionString);
        return db.ExecuteSprocAccessor<Request>("ResponseTracking_Get_Requests", requestId).ToList().FirstOrDefault();
    }

---

You also use this a lot inside that same class, make it a class scoped variable, move it outside of all those Methods.

it becomes this:

public static class DataAcessWrapper
    {
        public static string _connectionString = ConfigurationManager.ConnectionStrings["ConnectionString"].ConnectionString;

        private SqlDatabase db = new SqlDatabase(_connectionString);

        public static IEnumerable<Request> GetRequests(){
            return db.ExecuteSprocAccessor<Request>("ResponseTracking_Get_Requests");
        }

        public static Request GetRequest(int requestId)
        {
            return db.ExecuteSprocAccessor<Request>("ResponseTracking_Get_Requests", requestId).ToList().FirstOrDefault();
        }

        public static void SaveRequest(Request request)
        {
            DbCommand dbc = db.GetStoredProcCommand("ResponseTracking_Put_Request");
            db.AddInParameter(dbc, "@RequestID", DbType.Int32, request.RequestID);
            db.AddInParameter(dbc, "@Category", DbType.String, request.Category);
            db.AddInParameter(dbc, "@Title", DbType.String, request.Title);
            db.AddInParameter(dbc, "@DateReceived", DbType.DateTime2, request.DateReceived);
            db.AddInParameter(dbc, "@Source", DbType.String, request.Source);
            db.AddInParameter(dbc, "@ResponseRequired", DbType.String, request.ResponseRequired);
            db.AddInParameter(dbc, "@ResponseDateDue", DbType.DateTime2, request.ResponseDateDue);
            db.AddInParameter(dbc, "@ApprovalLevelRequired", DbType.String, request.ApprovalLevelRequired);
            db.AddInParameter(dbc, "@FollowUpDate", DbType.DateTime2, request.FollowUpDate);
            db.AddInParameter(dbc, "@Request", DbType.String, request.RequestDescription);
            db.AddInParameter(dbc, "@Status", DbType.String, request.Status);
            db.AddInParameter(dbc, "@Notes", DbType.String, request.Status);
            db.AddInParameter(dbc, "@ClosedDate", DbType.DateTime2, request.ClosedDate);
            db.AddInParameter(dbc, "@EmployeeID", DbType.String, request.EmployeeID);
            db.AddInParameter(dbc, "@CreationDateTime", DbType.DateTime2, request.CreationDateTime);
            db.ExecuteNonQuery(dbc);
        }

        public static void DeleteRequest(int requestId)
        {
            DbCommand dbc = db.GetStoredProcCommand("ResponseTracking_Delete_Request", requestId);
            db.ExecuteNonQuery(dbc);
        }
    }