C# Repository Design Pattern Advice

Hello experts!

I wrote an application for internal inventory purposes using Windows Forms and Linq to Sql classes. Recently I have been in the process of expanding the applications functionality which required some decoupling between the UI and the Data Access Layer.


At this point everything is working, but I would appreciate some feedback on my design implementation to see if I am on the right track, or possibly overly complicating things with too much abstraction or unnecessary code.

I know there is many different ways of doing this, but here is my pattern:

I have 3 projects in my solution:
- DataModel which contains my LinqToSql Classes and my DataContext

  Product class and ProductInventoryDataContext

- DataAccess which contain my Repositories to each Class

public class ContextConnection
{
        public ProductInventoryDataContext GetContext()
        {
            return new ProductInventoryDataContext();
        }
}

public interface IProductRepository
{
        Product GetProduct(Guid id);
        void SaveProduct(Product product);
        void DeleteProduct(Product product);
        IList<Product> GetAllProducts();
}

public class ProductRepository : RepositoryAbstract, IProductRepository
{
        private readonly ContextConnection _dbContext;

        public ProductRepository()
        {
            this._dbContext = new ContextConnection();
        }

        public Product GetProduct(Guid id)
        {
            Product result;
            using (var dc = _dbContext.GetContext())
            {
                result = dc.Products.First(prd => prd.Id.Equals(id));
            }

            return result;
        }

        // more CRUD methods...
}

Open in new window


- InventoryClient which contains an ApplicationService to the repositories and all of my Forms

- ApplicationService

    public abstract class AbstractApplicationService
    {
        public abstract IList<Product> GetProductInformation();
        // .... other methods
    }



/// <summary>
/// This is a singleton class which acts a a service between the Data Access Layer and the UI
/// </summary>
public class SerialInventoryService : AbstractApplicationService
{
        // singleton service object
        private static readonly SerialInventoryService _instance = new SerialInventoryService();

        // repository instances
        private IProductRepository _productRepository;

        /// <summary>
        /// Instantiate single instance and initialize repositories
        /// </summary>
        private SerialInventoryService()
        {
            InitializeRepositories();
        }

        /// <summary>
        /// Return singleton instance of Serial Service
        /// </summary>
        public static SerialInventoryService Instance
        {
            get { return _instance; }
        }

        /// <summary>
        /// Override from base class
        /// Initialize all repositories on singleton instantiation
        /// </summary>
        public override sealed void InitializeRepositories()
        {
            this._productRepository = new ProductRepository();
        }

        /// <summary>
        /// Override from base class
        /// Get a collection of products
        /// </summary>
        /// <returns>A list collection of products</returns>
        public override IList<Product> GetProductInformation()
        {
            return _productRepository.GetAllProducts().ToList<Product>();
        }
        
        // implement other abstract methods from base
}

Open in new window


And then in my Form I use the service as follows:

private void FrmParentMain_Load(object sender, EventArgs e)
{
            QueryCaptureInformation();
}
		
private void QueryCaptureInformation()
{
            try
            {
                var products = SerialInventoryService.Instance.GetProductInformation();
                var errors = SerialInventoryService.Instance.GetErrorInformation();

                if (products.Count <= 0)
                {
                    this.pnlBeginInventory.Enabled = false; // further querying held off
                }
                else
                {
                    this.pnlBeginInventory.Enabled = true; // enabling will allow further querying for inventory information

                    this.lblLastCaptureDate.Text = (products[0].CaptureDate.ToString());

                    this.lblTotalInventory.Text = (products.Count.ToString());

                    this.lblAWIInventoryCount.Text = (products.Count(x => x.Company.Equals(CoAwi)).ToString());
                    this.lblATIInventoryCount.Text = (products.Count(x => x.Company.Equals(CoAti)).ToString());

                    this.lblTotalScannedCount.Text = (products.Count(x => x.IsScanned).ToString());
                    this.lblAWIScannedCount.Text = (products.Count(x => x.IsScanned && x.Company.Equals(CoAwi)).ToString());
                    this.lblATIScannedCount.Text = (products.Count(x => x.IsScanned && x.Company.Equals(CoAti)).ToString());

                    this.lblErrorCount.Text = (errors.Count.ToString());
                }
            }
            catch (SqlException)
            {
                MessageBox.Show(Properties.Resources.Unexpected_Error_Message,
                                Properties.Resources.Connection_Error_Message,
                                MessageBoxButtons.OK, MessageBoxIcon.Error);
            }
            catch (Exception)
            {
                MessageBox.Show(Properties.Resources.Exception_Thrown_Message,
                                Properties.Resources.Exception_Message,
                                MessageBoxButtons.OK, MessageBoxIcon.Error);
            }
}

Open in new window


I have a future application that I would like to implement this same design into which will be on a larger scale with potentially 50-100 concurrent users, so any advice is greatly appreciated!
I_sAsked:
Who is Participating?
 
Dale BurrellConnect With a Mentor DirectorCommented:
Further points that come to mind:

- you could have a generic repository interface which has the basic methods, then each specific interface inherits from the generic one.

- you could have 2 generic repository interfaces, a read only one, and an edit one which inherits from the read only one.

- having your repository instanciate the data context could be a bad idea because if you obtain data from two different repositories the objects will exist in different data contexts and they won't be able to relate to each other. Most patterns I've seen make the data context a singleton with application scope.

- I would suggest that your applicationservice is unnecessary in this context - its not providing anything. If you actually want a service layer you would wrap a specific scope of logic in it, and really to be worth having you would have multiple service classes. Also I wouldn't use a singleton as a service. Either instanciate it and inject it into the application, or use a dependency injection tool to do it for you.

Thats my 2 cents worth :)
0
 
Dale BurrellDirectorCommented:
I used as my reference the book http://p2p.wrox.com/book-professional-asp-net-design-patterns-598/ - you can probably download the code without the book, although I found the book very useful in understanding the various options.

I also downloaded a few other examples of well implemented repository patterns in the internet.

One thing I will say is that I ended up exposing IQuerable from my repositories and the complexity to avoid that really didn't seem worth it after having spent a considerable amount of time researching it. I still mostly create a *proper* interface into the repository, but for searches and similar cases involving variable conditions, sorting and paging I access the IQueryable directly from my service.


I will try and comment further later, but check that wrox code out as it helped me a huge amount.
0
 
I_sAuthor Commented:
Thanks for your feedback, I am going to use your advice regarding the repositories and also remove the application service. The only thing I am not sure of is how to create the DataContext as a singleton because I am using generated code by LINQ to SQL classes..

Any pointers on that??

Thanks again!
0
 
Dale BurrellDirectorCommented:
The code download I recommended shows good ways to instanciate and use the datacontext, but in reality you've written a singleton for the application service, you just do something similar and all it does it create a datacontext once and then supply it e.g.

      private readonly ContextConnection _dbContext;

        public DataContextSingleton()
        {
            this._dbContext = new ContextConnection();
        }
0
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

All Courses

From novice to tech pro — start learning today.