3
\$\begingroup\$

I am beginning to use generics and would like feedback on the following set of classes that will serve as the basis for an app that contains a large number of basic CRUD-y entities.

I have used Car and Person classes as examples, but there will be a whole bunch of basic classes like these, perhaps 50 +.

My aim is to make maintenance / adding more of these classes as painless as possible, but also to allow for deviations when they arise. With that in mind, for the Person class I have:

  • added a custom action / viewmodel
  • added a different method signature for Index, that will allow filtering by the Starsign property

The Repository layer should be flexible and allow non-enumerated selections. The ViewModelProvider is the cut off point and all methods should return enumerated data.

I'd really appreciate your opinions on this code and any suggestions for improvement.

Entity Base Classes

public interface IDBEntity
{
    int ID { get; set; }
    string Title { get; set; }
    List<string> PreventDeleteReasons { get; }
    bool IsDeletable { get; }
}

public class DBEntity : IDBEntity
{
    [Key]
    [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
    public int ID { get; set; }

    [Display(Name = "Name",
        Description = "")]
    [StringLength(250, ErrorMessage = "{0} cannot be more than {1} characters long ")]
    public virtual string Title { get; set; }



    // just override this to prevent delete
    // protected: The type or member can only be accessed by code in the same class or struct, or in a derived class.
    protected virtual List<string> GetPreventDeleteReasons()
    {
        return new List<string>();
    }

    // no need to override these
    public virtual List<string> PreventDeleteReasons
    {
        get
        {
            if (_PreventDeleteReasons == null)
            {
                _PreventDeleteReasons = GetPreventDeleteReasons();
            }
            return _PreventDeleteReasons;
        }
    }

    [NotMapped]
    private List<string> _PreventDeleteReasons { get; set; }

    public virtual bool IsDeletable
    {
        get
        {
            return (PreventDeleteReasons.Count == 0);
        }
    }
}

Example Entities

public class Person : DBEntity, IDBEntity
{
    public virtual string Name { get; set; }
    public virtual string Starsign { get; set; }
    protected override List<string> GetPreventDeleteReasons()
    {
        List<string> reasons = base.GetPreventDeleteReasons();
        if (Starsign == "Aquarius")
        reasons.Add("Aquarians are too cool to be deleted");
        return reasons;
    }
}
public class Car : DBEntity, IDBEntity
{
    public virtual string Model { get; set; }
    public virtual string Colour { get; set; }
    protected override List<string> GetPreventDeleteReasons()
    {
        List<string> reasons = base.GetPreventDeleteReasons();
        reasons.Add("It's a really nice car");
        reasons.Add("You need to get to work every day");
        return reasons;
    }
}

Repository

public interface IGenericRepository<T> where T : DBEntity
{
    IQueryable<T> AllAsQueryable(params Expression<Func<T, object>>[] includeExpressions);
    List<T> AllAsList(params Expression<Func<T, object>>[] includeExpressions);
    List<T> SelectAll(params Expression<Func<T, object>>[] includeExpressions);
    List<T> SelectAllWhere(Expression<Func<T, bool>> whereBit, params Expression<Func<T, object>>[] includeExpressions);
    T SelectByID(int id, params Expression<Func<T, object>>[] includeExpressions);
    void Create(T obj);
    void Edit(T obj);
    void Delete(object id);
    void Save();
    void Dispose(bool disposing);
}


public class GenericRepository<T> : IGenericRepository<T> 
    where T : DBEntity
{
    private ApplicationDbContext DB = null;
    private DbSet<T> Table = null;


    public GenericRepository()
    {
        DB = new ApplicationDbContext();
        Table = DB.Set<T>();
    }



    public virtual IQueryable<T> AllAsQueryable(params Expression<Func<T, object>>[] includeExpressions)
    {
        return includeExpressions.Aggregate<Expression<Func<T, object>>, IQueryable<T>>
            (Table, (current, expression) => current.Include(expression));
    }

    public virtual List<T> AllAsList(params Expression<Func<T, object>>[] includeExpressions)
    {
        return includeExpressions.Aggregate<Expression<Func<T, object>>, IQueryable<T>>
         (Table, (current, expression) => current.Include(expression))
         .ToList();
    }



    public virtual List<T> SelectAll(params Expression<Func<T, object>>[] includeExpressions)
    {
        return AllAsQueryable(includeExpressions).ToList();
    }

    public virtual List<T> SelectAllWhere(Expression<Func<T, bool>> whereBit, params Expression<Func<T, object>>[] includeExpressions)
    {
        return AllAsQueryable(includeExpressions).Where(whereBit).ToList();
    }


    public virtual T SelectByID(int id, params Expression<Func<T, object>>[] includeExpressions)
    {
        if (includeExpressions.Any())
        {
            var set = includeExpressions.Aggregate<Expression<Func<T, object>>, IQueryable<T>>
                     (Table, (current, expression) => current.Include(expression));

            return set.SingleOrDefault(s => s.ID == id);
        }
        return Table.Find(id);
    }



    public virtual void Create(T obj)
    {
        Table.Add(obj);
        Save();
    }



    public virtual void Edit(T obj)
    {
        Table.Attach(obj);
        DB.Entry(obj).State = EntityState.Modified;
        Save();
    }



    public void Delete(object id)
    {
        T existing = Table.Find(id);
        Table.Remove(existing);
        Save();
    }



    public virtual void Save()
    {
        DB.SaveChanges();
        //Save();
    }



    public virtual void Dispose(bool disposing)
    {
        if (disposing)
        {
            DB.Dispose();
        }
    }
}

ViewModels

public interface IItemVM<T>
{
    T Item { get; set; }
}
public class GenericItemVM<T> : IItemVM<T>
    where T : DBEntity
{
    public T Item { get; set; }
}



public interface IItemListVM<T>
{
    List<T> Items { get; set; }
}
public class GenericItemListVM<T> : IItemListVM<T>
    where T : DBEntity
{
    public List<T> Items { get; set; }
}



public interface IAttemptDeleteVM<T> : IItemVM<T>
{
    int ID { get; set; }
    bool IsDeletable { get; }
    List<string> PreventDeleteReasons { get; set;  }
}
public class GenericAttemptDeleteVM<T> : GenericItemVM<T>, IAttemptDeleteVM<T>
    where T : DBEntity
{
    // p'less constructor populates PDR?
    public int ID { get; set; }
    public bool IsDeletable
    {
        get
        {
            return PreventDeleteReasons.Count == 0;
        }
    }
    public List<string> PreventDeleteReasons { get; set; }
}

Example Entities

public class PersonIndexVM : GenericItemListVM<Person>
{ }
public class PersonAquariansVM : GenericItemListVM<Person>
{
    public string Title { get; set; }
}
public class PersonDetailsVM : GenericItemVM<Person>
{ }
public class PersonCreateVM : GenericItemVM<Person>
{ }
public class PersonUpdateVM : GenericItemVM<Person>
{ }
public class PersonAttemptDeleteVM : GenericAttemptDeleteVM<Person>
{ }



public class CarIndexVM : GenericItemListVM<Car>
{ }
public class CarDetailsVM : GenericItemVM<Car>
{ }
public class CarCreateVM : GenericItemVM<Car>
{ }
public class CarUpdateVM : GenericItemVM<Car>
{ }
public class CarAttemptDeleteVM : GenericAttemptDeleteVM<Car>
{ }

ViewModel Providers

public interface IVMP<T, TIndexVM, TDetailsVM, TCreateVM, TUpdateVM, TAttemptDeleteViewModel>
    where T : DBEntity, IDBEntity
{
    GenericRepository<T> Repository { get; set; }
    TIndexVM Index();
    TCreateVM Create();
    TCreateVM Create(TCreateVM postedModel);
    TDetailsVM Details(int id);
    TUpdateVM Update(int id);
    TUpdateVM Update(TUpdateVM postedModel);
    TAttemptDeleteViewModel AttemptDelete(int id);
}
public abstract class GenericVMP<T, TIndexVM, TDetailsVM, TCreateVM, TUpdateVM, TAttemptDeleteVM> //: IVMP<T, TIndexVM, TDetailsVM, TCreateVM, TUpdateVM, TAttemptDeleteVM>
    where T : DBEntity, new()
    where TIndexVM : class, IItemListVM<T>, new()
    where TDetailsVM : class, IItemVM<T>, new()
    where TCreateVM : class, IItemVM<T>, new()
    where TUpdateVM : class, IItemVM<T>, new()
    where TAttemptDeleteVM : class, IAttemptDeleteVM<T>, new()
{
    // shd be public? if not kill from iface
    public GenericRepository<T> Repository { get; set; }

    public GenericVMP()
    {
        Repository = new GenericRepository<T>();
    }
    public TIndexVM Index()
    {
        // all to list, so need repo
        return new TIndexVM()
        {
            Items = Repository.AllAsList()
        };
    }
    public TDetailsVM Details(int id)
    {
        return new TDetailsVM()
        {
            Item = Repository.SelectByID(id)
        };
    }
    public TCreateVM Create()
    {
        return new TCreateVM()
        {
            Item = new T()
        };
    }
    public TCreateVM Create(TCreateVM postedModel)
    {
        return new TCreateVM()
        {
            Item = postedModel.Item
        };
    }
    public TUpdateVM Update(int id)
    {
        return new TUpdateVM()
        {
            Item = Repository.SelectByID(id)
        };
    }
    public TUpdateVM Update(TUpdateVM postedModel)
    {
        return new TUpdateVM()
        {
            Item = postedModel.Item
        };
    }
    public TAttemptDeleteVM AttemptDelete(int id)
    {
        var item = Repository.SelectByID(id);
        return new TAttemptDeleteVM()
        {
            ID = id,
            Item = item,
            PreventDeleteReasons = item.PreventDeleteReasons
        };
    }
}

Example Entities

public class PersonVMP : GenericVMP<Person, PersonIndexVM, PersonDetailsVM, PersonCreateVM, PersonUpdateVM, PersonAttemptDeleteVM>,  IVMP<Person, PersonIndexVM, PersonDetailsVM, PersonCreateVM, PersonUpdateVM, PersonAttemptDeleteVM>
{

    public PersonAquariansVM Aquarians()
    {
        return new PersonAquariansVM()
        {
            Title = "Viewing Just Aquarians",
            Items = Repository.SelectAllWhere(p => p.Starsign == "Aquarius").ToList()
        };
    }

    public PersonIndexVM CustomIndex(string id)
    {
        return new PersonIndexVM()
        {
            Items = Repository.SelectAllWhere(m => m.Starsign == id).ToList()
        };
    }
}
public class CarVMP : GenericVMP<Car, CarIndexVM, CarDetailsVM, CarCreateVM, CarUpdateVM, CarAttemptDeleteVM>, IVMP<Car, CarIndexVM, CarDetailsVM, CarCreateVM, CarUpdateVM, CarAttemptDeleteVM>
{ }

DBUpdaters

public interface IDBU<T, TCreateVM, TUpdateVM, TAttemptDeleteVM>
{
    void Create(TCreateVM model);
    void Update(TUpdateVM model);
    bool AttemptDelete(TAttemptDeleteVM model);
}
public abstract class GenericDBU<T, TCreateVM, TUpdateVM, TAttemptDeleteVM> //: IDBU<T, TCreateVM, TUpdateVM, TAttemptDeleteVM>
    where T : DBEntity, new()
    where TCreateVM : class, IItemVM<T>, new()
    where TUpdateVM : class, IItemVM<T>, new()
    where TAttemptDeleteVM : class, IAttemptDeleteVM<T>, new()
{
    // is ok private?
    private GenericRepository<T> Repository { get; set; }


    public GenericDBU()
    {
        Repository = new GenericRepository<T>();
    }

    public void Create(TCreateVM model)
    {
        Repository.Create(model.Item);
    }
    public void Update(TUpdateVM model)
    {
        Repository.Edit(model.Item);
    }
    public bool AttemptDelete(TAttemptDeleteVM model)
    {
        if (model.Item.IsDeletable)
        {
            Repository.Delete(model.ID);
            return true;
        }
        return false;
    }
}

Example Entities

public class PersonDBU : GenericDBU<Person, PersonCreateVM, PersonUpdateVM, PersonAttemptDeleteVM>, IDBU<Person, PersonCreateVM, PersonUpdateVM, PersonAttemptDeleteVM>
{ }
public class CarDBU : GenericDBU<Car, CarCreateVM, CarUpdateVM, CarAttemptDeleteVM>, IDBU<Car, CarCreateVM, CarUpdateVM, CarAttemptDeleteVM>
{ }

Controllers

public class GenericController<T, TVMP, TIndexVM, TDetailsVM, TCreateVM, TUpdateVM, TAttemptDeleteVM, TDBU> : Controller
    where T : DBEntity, new()
    where TVMP : IVMP<T, TIndexVM, TDetailsVM, TCreateVM, TUpdateVM, TAttemptDeleteVM>, new()
    where TIndexVM : class, new()
    where TDetailsVM : class, new()
    where TCreateVM : class, new()
    where TUpdateVM : class, new()
    where TAttemptDeleteVM : class, new()
    where TDBU : IDBU<T, TCreateVM, TUpdateVM, TAttemptDeleteVM>, new()
{


    private GenericRepository<T> Repository { get; set; }
    private IVMP<T, TIndexVM, TDetailsVM, TCreateVM, TUpdateVM, TAttemptDeleteVM> VMP { get; set; }
    private IDBU<T, TCreateVM, TUpdateVM, TAttemptDeleteVM> DBU { get; set; }


    public GenericController()
    {
        VMP = new TVMP();
        DBU = new TDBU();
        Repository = new GenericRepository<T>();
    }




    public virtual ActionResult Index()
    {
        TIndexVM vm = VMP.Index();
        return View(vm);
    }


    public virtual ActionResult Details(int id)
    {
        TDetailsVM vm = VMP.Details(id);
        return View(vm);
    }


    public virtual ActionResult Create()
    {
        var vm = VMP.Create();
        return View(vm);
    }

    [HttpPost]
    [ValidateAntiForgeryToken]
    public virtual ActionResult Create(TCreateVM postedModel)
    { 
        if (ModelState.IsValid)
        {
            DBU.Create(postedModel);
            return RedirectToAction("Index");
        }
        return View(postedModel);
    }




    public virtual ActionResult Edit(int id)
    {
        var vm = VMP.Update(id);
        return View(vm);
    }

    [HttpPost]
    [ValidateAntiForgeryToken]
    public virtual ActionResult Edit(TUpdateVM postedModel)
    {
        if (ModelState.IsValid)
        {
            DBU.Update(postedModel);
            return RedirectToAction("Index");
        }
        return View(postedModel);
    }


    [ActionName("Delete")]
    public virtual ActionResult AttemptDelete(int id)
    {
        var vm = VMP.AttemptDelete(id);
        return View(vm);
    }

    // POST: People/Delete/5
    [HttpPost, ActionName("Delete")]
    [ValidateAntiForgeryToken]
    public virtual ActionResult DeleteConfirmed(int id)
    {
        var vm = VMP.AttemptDelete(id);
        bool success = DBU.AttemptDelete(vm);
        if (success)
        {
            return RedirectToAction("Index");
        }
        return View("AttemptDelete", vm);
    }
}

Example Entities

public class PeopleController : GenericController<Person, PersonVMP, PersonIndexVM, PersonDetailsVM, PersonCreateVM, PersonUpdateVM, PersonAttemptDeleteVM, PersonDBU>
{
    private ApplicationDbContext db = new ApplicationDbContext();


    // custom methods here

    private PersonVMP PersonVMP = new PersonVMP();

    public ActionResult Aquarians()
    {
        var vm = PersonVMP.Aquarians();
        return View(vm);
    }


    [NonAction]
    public override ActionResult Index()
    {
        throw new SystemException();
    }

    public ActionResult Index(string id)
    {
        PersonIndexVM vm;
        if (String.IsNullOrEmpty(id))
        {
            vm = PersonVMP.Index();
        }
        else
        {
            vm = PersonVMP.CustomIndex(id);
        }
        return View(vm);
    }
}



public class CarsController : GenericController<Car, CarVMP, CarIndexVM, CarDetailsVM, CarCreateVM, CarUpdateVM, CarAttemptDeleteVM, CarDBU>
{
    private ApplicationDbContext db = new ApplicationDbContext();


    // custom methods here
}
\$\endgroup\$
2
\$\begingroup\$

IDBEntity, IGenericRepository<T>, IItemListVM<T>, IAttemptDeleteVM<T>

You are exposing a List<string> via a property which shouldn't been done. Assume you want/need to change this to another type which say is implementing IList<T> or ICollection<T> but isn't a List<T> then the caller/user of your interface would need to change his/her implementation. A List<T> is an implementation detail which shouldn't be exposed.

See also: CA1002: Do not expose generic lists

DBEntity

// no need to override these
public virtual List<string> PreventDeleteReasons  

so why is this property virtual ? If it shouldn't be able to be overridden then remove the virtual keyword and also that comment.

General##

Based on the Net naming guidelines variables should be named using camelCase casing. See DB and Table in GenericRepository<T>.

\$\endgroup\$
  • \$\begingroup\$ thank you - that's very helpful. Now making those changes \$\endgroup\$ – Martin Hansen Lennox Jan 12 '16 at 14:54

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.