Is it proper form to extend a model object (e.g. Product) and add a Create() method that inserts into the database? (MVC 5 Entity Framework 6)

Question

So I am currently extending the classes that Entity Framework automatically generated for each of the tables in my database. I placed some helpful methods for processing data inside these partial classes that do the extending.

My question, however, is concerning the insertion of rows in the database. Would it be good form to include a method in my extended classes to handle this?

For example, in the Product controller's Create method have something like this:

[HttpPost]
public ActionResult Create(Product p)
{
    p.InsertThisProductIntoTheDatabase();  //my custom method for inserting into db
    return View();
}

Something about this feels wrong to me, but I can't put my finger on it. It feels like this functionality should instead be placed inside a generic MyHelpers.cs class, or something, and then just do this:

var h = new MyHelpers();
h.InsertThisProductIntoTheDatabase(p);

What do you guys think? I would prefer to do this the "correct" way.

MVC 5, EF 6

edit: the InsertThisProductIntoTheDatabase method might look something like:

public partial class Product()
{

    public void InsertThisProductIntoTheDatabase()
    {
         var context = MyEntities();

         this.CreatedDate = DateTime.Now;
         this.CreatedByID = SomeUserClass.ID;
         //some additional transformation/preparation of the object's data would be done here too.  My goal is to bring all of this out of the controller.

         context.Products.Add(this);
    }

}

what was wrong with context.Products.Add(p);? what does InsertThisProductIntoTheDatabase do? — Fran, Aug 11 '16 at 20:20
Good question, here is my answer: if I just rely on a method built in to the object's base class, then I dont' have to deal with creating context, etc. in the controller. I simply have one line: p.InsertThisProductIntoTheDatabase() to handle all of the insertion logic. Taken further, I sometimes have to do a little behind-the-scenes transformation work on an object's data before I insert it into the database too, so this code could be moved out of the controller too (e.g. set the object's CreateBy and CreatedDate columns) — M. Smith, Aug 11 '16 at 20:28
can you update the question with the code to InsertThisProductIntoTheDatabase() — Fran, Aug 11 '16 at 20:39

score 1 · Answer 1 · answered Aug 11 '16 at 20:20

1

Why don't you simply use:

db.Products.Add(p);
db.SaveChanges();

Your code would be much cleaner and it will certainly be easier for you to manage it and get help in the future. Most of samples available in internet use this schema. Extension methods and entities does not look pleasnt.

BTW: Isn't InsertThisProductIntoTheDatabase() method name too long?

answered Aug 11 '16 at 20:20

mateudu

108
11

1

Good question, here is my answer: if I just rely on a method built in to the object's base class, then I dont' have to deal with creating context, etc. in the controller. I simply have one line: p.InsertThisProductIntoTheDatabase() to handle all of the insertion logic. Taken further, I sometimes have to do a little behind-the-scenes transformation work on an object's data before I insert it into the database too, so this code could be moved out of the controller too (e.g. set the object's CreateBy and CreatedDate columns). Also, yes that is a long name- was only for the example. – M. Smith Aug 11 '16 at 20:29

score 1 · Accepted Answer · edited May 23 '17 at 12:08

One of the problems I see is that the entity framework DBContext is a unit of work. if you create a unit of work on Application_BeginRequest when you pass it into controller constructor it acts as a unit of work for the entire request. maybe it's only updating 1 entity in your scenario, but you could be writing more information to your database. unless you are wrapping everything in a TransactionScope, all these Saves are going to be independent which could leave your database in an inconsistent state. And even if you are wrapping everything with a TransactionScope, I'm pretty sure that transaction is going to be promoted to the DTC because you are making multiple physical connections in a single controller and sql server isn't that smart.

Going the BeginRequest route seems like less work than adding methods to all of your entities to save itself. Another issue here is that an EF entity is supposed to be a not really know anything about it's own persistence. That's what the DbContext is for. So putting a reference back to the DbContext breaks this isolation.

Your second reason, adding audit information to the entity, again adding this to each entity is a lot of work. You could override SaveChanges on the context and do it once for every entity. See this SO answer.

By going down this road I think that you are breaking SOLID design principles because your entities violate SRP. introduce a bunch of cohesion and you are ending up writing more code than you need. So i'd advocate against doing it your way.

Is it proper form to extend a model object (e.g. Product) and add a Create() method that inserts into the database? (MVC 5 Entity Framework 6)

2 Answers2