I have the following class:
public class SectionViewModel
{
private static EFModels.VTSEntities _db = new EFModels.VTSEntities();
public int ID { get; set; }
public string SectionName { get; set; }
public bool Active { get; set; }
public string SiteName { get; set; }
}
I want to select one element from _db.Sections and fill object of this class. I can do it like it:
public SectionViewModel(int ID)
{
var s = (from i in _db.Sections
where i.ID == ID
select new SectionViewModel()
{
ID = i.ID,
SectionName = i.SectionName,
Active = i.Active,
SiteName = i.Site.SiteName
}).FirstOrDefault();
ID = s.ID;
SectionName = s.SectionName;
Active = s.Active;
}
It works, but when count of fields is tens, code is huge. I would like to write something similar
// IT DOES NOT WORK, ONLY EXAMPLE
public SectionViewModel(int ID)
{
this = (from i in _db.Sections
where i.ID == ID
select new SectionViewModel()
{
ID = i.ID,
SectionName = i.SectionName,
Active = i.Active,
SiteName = i.Site.SiteName
}).FirstOrDefault();
}
ADDED:
Create SectionViewModel object (it's a View model class):
public ActionResult SectionForm(int? id)
{
SectionViewModel model = new SectionViewModel(id);
return View(model);
}
but, of course, it's impossible because "this" is available only for read. Any way to do it?
Edit
OK, so you construct a SectionViewModel, not a Section, from a Section. That makes some difference, but still many remarks from my original answer apply.
A better way to do this is
public ActionResult SectionForm(int id)
{
Section section = this._context.Sections.Find(id);
SectionViewModel model = .... // Mapping code
return View(model);
}
This part // Mapping code can be anything that copies properties from section to model. You could use AutoMapper.
The reason not to do this in SectionViewModel's constructor is that there shouldn't be a static context there in the first place. You could create and dispose a context in stead. But who says that SectionViewModels are always constructed individually? Maybe in another method you will return a list of them. Creating each model separately would be highly inefficient. AutoMapper's Project().To approach would be appropriate there.
Original text
It's absurd to construct an object in a constructor (which is what happens by the part var s = (...).FirstOrDefault()) and then copy its properties to the owner of the constructor, which is the same type, Section. It's even more absurd that in the query you also construct a Section from a section. So after running the statement...
Section model = new Section(id);
...you have constructed three identical Sections of which the last one is finally used:
Section 1: from i in _db.Sections where i.ID == ID select i.
Section 2: select new Section() {...}
Section 3: Section(int ID)
And EF doesn't even allow a statement like
from i in _db.Sections
select new Section() {...}
It will tell you that you can't construct an entity in an LINQ-to-Entities query.
But removing this select new Section() {...} is not even the beginning of a sound refactoring. The whole construct is absurd.
Another bad practice is to have a static context. Contexts are designed to have a short lifespan, because they cache each entity they fetch from the database. A static context is a memory leak.
The way to achieve what you want is simply...
public ActionResult SectionForm(int id)
{
Section model = this._context.Sections.Find(id);
return View(model);
}
...where this._context is an instance of your VTSEntities that is created per controller instance (or injected into it by an Inversion of Control container).
Your programming style is faintly reminiscent of Active Record, a pattern that doesn't mix well with EF's repository/unit of work pattern (where DbSet is a repo and a DbContext is a UoW). It also breaks the persistance ignorance principle that EF is built on. It breaks query composability and it easily causes n + 1 queries.
