#StackBounty: #c# #generics #inheritance #asp.net-mvc #repository E-commerce site for posting advertisements

Bounty: 50

I am developing an e-commerce website in ASP.NET MVC. Users can post advertisements of different types on the site.

I am using inheritance to define my Ad types, and this review is mainly about taking advantage of the hierarchical structure to remove repeated code in Controllers and Views.

I have different Ad types: SimpleAd, Car and RealEstateRental.

Every Ad is derived from AdBase which has all the common properties:

public abstract class AdBase
{
    public long AdBaseId { get; set; }
    public bool IsActive { get; set; }
    public long UserId { get; set; }
    public string Title { get; set; }
    public short AdDurationInDays { get; set; }
    public string PhotosFolder { get; set; }
}

Now other Ads are derived from this base class:

public class SimpleAd : AdBase
{
    public decimal Price { get; set; }
}

public class Car : AdBase
{
    public decimal Price { get; set; }
    public string Make { get; set; }
}

public class RealEstateRental : AdBase
{
    public decimal WeeklyRent { get; set; }
    public DateTime AvailableFrom { get; set; }
    public short NoOfBedrooms { get; set; }
    public short NoOfBathrooms { get; set; }
}

I am using Entity Framework to interact with database and I am using Unit of Work and Repository patterns:

I have an Abstract base repository (the intention is to avoid rewriting the same function for each Ad type, so I have put the common functions here):

public interface IAdBaseRepository<TEntity> where TEntity : AdBase
{
    TEntity Get(long adBaseId);
}

public abstract class AdBaseRepository<TEntity> : IAdBaseRepository<TEntity> where TEntity : AdBase
{
    public AdBaseRepository(ApplicationDbContext context) : base(context)
    {
    }

    public TEntity Get(long adBaseId)
    {
        return Context.AdBase.OfType<TEntity>()
                  .Where(r => r.IsActive == true && r.AdBaseId == adBaseId)
                  .FirstOrDefault();
    }
}

Other ad repositories inherit from the above class:

public interface ISimpleAdRepository : IAdBaseRepository<SimpleAd>
{
}

public class SimpleAdRepository : AdBaseRepository<SimpleAd>,
    ISimpleAdRepository
{
    public SimpleAdRepository(ApplicationDbContext context) : base(context)
    {
    }
}

public interface ICarRepository : IAdBaseRepository<Car>
{
}

public class CarRepository : AdBaseRepository<Car>,
    ICarRepository
{
    public CarRepository(ApplicationDbContext context) : base(context)
    {
    }
}

And this is my Unit of Work:

public class UnitOfWork : IUnitOfWork 
{
    protected readonly ApplicationDbContext Context;

    public UnitOfWork(ApplicationDbContext context)
    {
        Context = context;
        SimpleAd = new SimpleAdRepository(Context);
        RealEstateRental = new RealEstateRentalRepository(Context);
        Car = new CarRepository(Context);
    }

    public ISimpleAdRepository SimpleAd { get; private set; }
    public IRealEstateRentalRepository RealEstateRental { get; private set; }
    public ICarRepository Car { get; private set; }

    public int SaveChanges()
    {
        return Context.SaveChanges();
    }

    public void Dispose()
    {
        Context.Dispose();
    }
}

I am happy with everything so far… but the problem is I don’t know how I can take advantage of this inheritance hierarchy in my Controllers and Views.

At the moment, I have 3 Controllers: SimpleAdController, CarController and RealEstateRentalController:

public class SimpleAdController : ControllerBase
{
    private IUnitOfWork _unitOfWork;

    public SimpleAdController(IUnitOfWork unitOfWork)
    {
        _unitOfWork = unitOfWork;
    }

    [HttpGet]
    public ActionResult Display(long id)
    {
        SimpleAd simpleAd = _unitOfWork.SimpleAd.Get(id);
        /* 
         * I have not included my ViewModel Classes in this code review to keep
         * it small, but the ViewModels follow the same inheritance pattern
         */
        var simpleAdDetailsViewModel = Mapper.Map<SimpleAdDetailsViewModel>(simpleAd);
        return View(simpleAdDetailsViewModel);
    }
}

CarController and RealEstateRentalController have similar display function, except the type of the Ad is different (e.g. in CarController I have):

    public ActionResult Display(long id)
    {
        Car car = _unitOfWork.Car.Get(id);
        var carViewModel = Mapper.Map<CarViewModel>(car);
        return View(car);
    }

What I wanted to achieve was to create an AdBaseController to put all the common methods in it, something like this:

public class AdBaseController : ControllerBase
{
    private IUnitOfWork _unitOfWork;

    public AdBaseController(IUnitOfWork unitOfWork)
    {
        _unitOfWork = unitOfWork;
    }

    // Display for generic ad type
    [HttpGet]
    public ActionResult Display(long id)
    {
        // SimpleAd simpleAd = _unitOfWork.SimpleAd.Get(id);
        /* 
         * I need to replace the above line with a generic ad type... 
         * something like: _unitOfWork<TAd>.GenericAdRepository.Get(id)
         */

        // var simpleAdDetailsViewModel = Mapper.Map<SimpleAdDetailsViewModel>(simpleAd);
        // return View(simpleAdDetailsViewModel);
        /* 
         * similarly I have to replace the above 2 lines with a generic type
         */
    }
}

If I do the above, then my Ad Controllers can inherit from it and I don’t need to repeat the same Display Method in every one of them… but then I need to make my UnitOfWork generic… or have 2 UoW (generic and non-generic)… which I am not sure if it is a good idea? Any recommendation on having a AdBaseController?


Similarly I am repeating a lot of code in my Views. For example, this is the display SimpleAdView:

@Html.Partial("DisplayAd/_Photos", Model)
@Model.Title
$@Model.Price
@Html.Partial("DisplayAd/_AdBaseHeading", Model) </div> </div> @Html.Partial("DisplayAd/_Description", Model)

And this is my display CarView:

@Html.Partial("DisplayAd/_Photos", Model)
@Model.Title
$@Model.Price
@Model.Make
@Html.Partial("DisplayAd/_AdBaseHeading", Model) </div> </div> @Html.Partial("DisplayAd/_Description", Model)

Again, I feel like I am repeating a lot of code in each view. I have tried to reduce the amount of repeated code by putting them in common Partial Views. I am not sure if there is a better way to do this?


Get this bounty!!!

Leave a Reply

This site uses Akismet to reduce spam. Learn how your comment data is processed.