0
\$\begingroup\$

I am creating a .NET Framework MVC EF application for PC components. I wanted to make full use of inheritance, polymorphism and generic repository pattern - but was wondering if my initial design implementation is the best way to go about it.

The major sections of the codebase are below:


Repository

I wrote this intending for it to be a generic repository for each component to call to retrieve records from the db, but as you'll see in the controller class I end up declaring several repository instances anyway.

ComponentRepository.cs

public class ComponentsRepository<T> : IComponentRepository<T> where T : class, IComponent
    {
        private readonly ApplicationDbContext _context;
        private DbSet<T> table = null;

        public ComponentsRepository()
        {
            _context = new ApplicationDbContext();
            table = _context.Set<T>();
        }

        public ComponentsRepository(ApplicationDbContext context)
        {
            _context = context;
            table = context.Set<T>();
        }

        public void Delete(int id)
        {
            table.Remove(GetById(id));
        }

        public IEnumerable<T> GetAll()
        {
            return table;
        }

        public T GetById(int id)
        {
            return table.Find(id);
        }

        public void Insert(T component)
        {
            table.Add(component);
        }

        public void Save()
        {
            _context.SaveChanges();
        }

        public void Update(T component)
        {
            table.Attach(component);
            _context.Entry(component).State = EntityState.Modified;
        }

        private bool disposed = false;
        protected virtual void Dispose(bool disposing)
        {
            if (!this.disposed)
            {
                if (disposing)
                {
                    _context.Dispose();
                }
            }
        }

        public void Dispose()
        {
            Dispose(true);
            GC.SuppressFinalize(this);
        }
    }

Component Models

I require all components to have a shared parent class for grouping in lists, basket, etc. Each component has unique properties that act as the specifications. (e.g. Core clock)

Component.cs

public class Component : Interfaces.IComponent
    {
        [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
        public int Id { get; set; }
        [DisplayName("Name")]
        public string Name { get; set; }
        [DisplayName("Description")]
        public string Description { get; set; }
        [DisplayName("Price")]
        public decimal Price { get; set; }

        public Manufacturer Manufacturer { get; set; }
        [DisplayName("Component Type")]
        public virtual ComponentType ComponentType => ComponentType.NoType;
        public virtual string GetComponentTypeLink => "";
    }

CPU.cs

public class CPU : Component
    {
        [DisplayName("Component Type")]
        public override ComponentType ComponentType => ComponentType.CPU;
        public override string GetComponentTypeLink => new UrlHelper(HttpContext.Current.Request.RequestContext).Action("CPU", "Products");

        [DisplayName("Core Count")]
        public int CoreCount { get; set; }
        [DisplayName("Core Clock")]
        public string CoreClock { get; set; }
    }

Storage.cs

public class Storage : Component

{
    [DisplayName("Component Type")]
    public override ComponentType ComponentType => ComponentType.Storage;
    public override string GetComponentTypeLink => new UrlHelper(HttpContext.Current.Request.RequestContext).Action("Storage", "Products");

    [DisplayName("Size")]
    public decimal Size { get; set; }
    [DisplayName("Read Speed")]
    public int ReadSpeed { get; set; }
    [DisplayName("Write Speed")]
    public int WriteSpeed { get; set; }
}

Controller

ComponentsController.cs

public class ComponentsController : Controller
    {
        private readonly IComponentRepository<CPU> _CPURepository;
        private readonly IComponentRepository<Storage> _StorageRepository;
        private readonly IComponentRepository<CPUCooler> _CPUCoolerRepository;
        private readonly IComponentRepository<Memory> _MemoryRepository;
        
        public ComponentsController()
        {
            _CPURepository = new ComponentsRepository<CPU>();
            _StorageRepository = new ComponentsRepository<Storage>();
            _CPUCoolerRepository = new ComponentsRepository<CPUCooler>();
            _MemoryRepository = new ComponentsRepository<Memory>();

        }

        public ActionResult Index()
        {
            // Simulate list, basket, checkout, etc.
            List<IComponent> componentList = new List<IComponent>();
            componentList.AddRange(_CPURepository.GetAll());
            componentList.AddRange(_StorageRepository.GetAll());
            componentList.AddRange(_CPUCoolerRepository.GetAll());
            componentList.AddRange(_MemoryRepository.GetAll());

            return View(componentList);
        }

        public ActionResult Details()
        {
            // Select view based on component type to view each sub-class specification

            return View();
        }
    }

Within the controller I'm creating a new repository instance for each concrete component implementation which gives of quite a bad smell. I don't know if this design would be elegant in handling CRUD functionalities down the line.

Any comments or insight would be greatly appreciated, thanks!

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Ask yourself the question what you gain by this repository layer. Current consensus leans to: nothing. Just use a context. Also, please mention the exact EF version and show the database model. It's important to know how the class model maps to the database model. \$\endgroup\$ Nov 3, 2021 at 12:51
  • \$\begingroup\$ Is this .NET or .NET Core? The feature set for EF and EF Core has not always been the same, and entity type inheritance has historically been different between the two. \$\endgroup\$
    – Flater
    Nov 3, 2021 at 15:45
  • \$\begingroup\$ @Flater .NET (4.7.2) - Sorry I forgot to mention \$\endgroup\$ Nov 3, 2021 at 16:09

1 Answer 1

0
\$\begingroup\$

but as you'll see in the controller class I end up declaring several repository instances anyway.

Whenever you use a middle-ware that uses a design pattern, try to not enclose it with the same design pattern, because it's assumed that design pattern has shipped the middle-ware core functionality at its simplest form. So, there is no actual advantage of trying to simplify what's already simplified.

Having said that, DbSet<T> in Entity Framework is actually a repository, and the DbContext is a UnitOfWork. So, doing a repository of another repository would not give any advantages, and it would actually add more complexity to your work. That's why EF has given some optionality to some features where you can customize them (like overriding SaveChanges or customizing the CRUD operations instead of using the pre-defined ones (Add, Remove ..etc).

However, with Entity Framework, you can use Service design pattern if you have some custom business logic, or you can create IDbSet extensions (or both).

If you use a Service then you only need to pass that service to the controller. A service can hold multiple repositories. so your final controller would be something like this :

public class ComponentsController : Controller
{
    private readonly ComponentService _service;
    
    public ComponentsController()
    {
        _service = new ComponentService();
    }

    public ActionResult Index()
    {
        // Simulate list, basket, checkout, etc.
        List<IComponent> componentList = _service.GetComponents();
        return View(componentList);
    }
}

For the models, inheritance is a good approach, however, when using models, it's better to unify the model purpose or role. In your case, you're using models for the database which would represent database schemas. Yet, you're mixing DataAnnotations attributes with other attributes that are not part of acceptable attributes to Entity Framework. same thing goes to GetComponentTypeLink property, which I think would be used on Views (or Controllers). So, keep the data models for the data layer, and create models to these models where needed. (e.g. view model, dto, poco ..etc.). The reason is basically you want to stabilize the data models, and have custom models from it, to be more maintainable.

The ComponentType is redundant, because you can achieve the goal using typeof(class).

Finally, the class naming is too general, you need to follow a better naming convention that would describe the class role something like HardwareComponent and for derived class CPUHardwareComponent and so on.

\$\endgroup\$
2
  • \$\begingroup\$ "So, doing a repository of another repository would not give any advantages" As much as I am a fan of not uow/repository-wrapping EF; it's not that there are no advantages, it's that the advantage usually isn't worth the effort. If you need your domain to be independent of EF, then there is value to wrapping it. However, this is often dogmatically done in cases where there is no real expectation of ever moving away from EF, at which point the wrapper is a wasted effort. \$\endgroup\$
    – Flater
    Nov 3, 2021 at 15:48
  • \$\begingroup\$ Thanks for you detailed response. It's clearer to see that a repository pattern isn't necessarily always beneficial and I might have abstracted the problem more than needed. I did look into Entity Framework strategies after posting and found the 'Table per Type' strategy to be exactly what I was looking for - in terms of implementing the base class as its own table that then derived class' tables hold keys for in order to achieve relational mapping. \$\endgroup\$ Nov 3, 2021 at 16:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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