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 aSection
, from aSection
. That makes some difference, but still many remarks from my original answer apply.A better way to do this is
This part
// Mapping code
can be anything that copies properties fromsection
tomodel
. 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 thatSectionViewModel
s are always constructed individually? Maybe in another method you will return a list of them. Creating each model separately would be highly inefficient. AutoMapper'sProject().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 aSection
from a section. So after running the statement......you have constructed three identical
Section
s 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
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...
...where
this._context
is an instance of yourVTSEntities
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 aDbContext
is a UoW). It also breaks the persistance ignorance principle that EF is built on. It breaks query composability and it easily causesn + 1
queries.