Reputation: 3410
I am new to development and trying to retrieve a record to display and edit in a FormView on an ASP.NET web app page, using Entity Framework 5.0 (database first method), but I am not sure of the best way to go about this.
To retrieve the record, I use the following code:
protected void Page_Load(object sender, EventArgs e)
{
LoadData(int.Parse(Session["PersonID"].ToString()));
}
private void LoadData(int iPersonID)
{
using (PeopleEntities ctx = new PeopleEntities())
{
var query = (from a in ctx.People
where a.PersonID == iPersonID
select a).FirstOrDefault();
TextBoxFirstName.Text = query.FirstName;
TextBoxLastName.Text = query.LastName;
}
}
And to save it, I use:
protected void ButtonSave_Click(object sender, EventArgs e)
{
SaveEmployee(int.Parse(Session["PersonID"].ToString()));
}
private void SaveEmployee(int iPersonID = 0)
{
using (PeopleEntities ctx = new PeopleEntities())
{
var query = (from a in ctx.People
where a.PersonID == iPersonID
select a).FirstOrDefault();
query.FirstName = TextBoxFirstName.Text;
query.LastName = TextBoxLastName.Text;
ctx.SaveChanges();
}
}
It seems silly to me to have these two methods each query the database to retrieve and update the record, but, again, I am a novice, and maybe I'm just missing something. Is there a way to populate the controls on the FormView with the entity and have a method to save the record without having to manually assign the values (query.FirstName = TextBoxFirstName.Text, etc.) based on state?
I have seen the EntityDataSource, but I do not think that will be a good option for anything but the simplest of things.
Can anyone please tell me if what I am doing is ok or provide a better example or guidance?
Your help is greatly appreciated!
Upvotes: 1
Views: 17408
Reputation: 8065
try using
xxx.xxx.SelectSingleOrDefault(c => c.AccountSenderID == userId_int)
replaces the use of anonymous lambda expressions (using var for instance)
xxx.xxx.Select(c => new { c.FriendInvitationID,c.AccountSenderID,
c.Account1.AccountID, c.Account1.FirstName, c.Account1.LastName, c.Account1.Email,
c.FriendInvitationStatus, c.CreationDate })
.Where(c => c.AccountSenderID == userId_int).ToList();
you dont have to describe your object even though anonymous is more dynamic in that sense (image you want to retrieve a json object with two dfferent references of the same table, in that case you must declare fields because they will have same names, just a though)
Upvotes: 0
Reputation: 22595
"It seems silly to me to have these two methods each query the database to retrieve and update the record"
You are absolutely correct Don't Repeat Yourself should be a mantra principal that you should endeavour to follow.
Here you have chosen to retrieve the data in the page load event and retrieve it again in the button click event. Both these events are happening within the same instance of a WebPage. You could store it in an instance variable and re-use it in the button click, or you could set up a property for the entity that is "lazy loaded". There are all sorts of ways of doing it. Lazy loading is probably definitely overkill here because you will probably only use the property in the PageLoad should understand when it is necessary to go to the database and when it isn't.
It is necessary to go to the database to get the data you want to display when the page is first loaded. Thereafter the data is normally present in form values when the page posts back.
It is also necessary to go to the database when you are updating a record - which happens in this example when your user clicks on the save button.
Here is an example of lazy loading something i probably shouldn't have mentioned:
private People _Person;
//lazy loaded property
private People Person
{
get
{
if (_Person == null)
using (PeopleEntities ctx = new PeopleEntities())
_Person = GetPerson(ctx);
//returning a Person that isn't updateable because we've disposed of the context
return _Person;
}
}
//Retrieve an updateable person
private static object GetPerson(PeopleEntities ctx)
{
return (from a in ctx.People
where a.PersonID == int.Parse(Session["PersonID"]
select a).FirstOrDefault();
}
The other problem your code has is that you always set the TextBoxes in the PageLoad event from the values in the database. This means that when you get to the ButtonSave_Click
event the values posted back have been overwritten by what was in the database and the changes won't be saved!.
So you should do this instead:
protected void Page_Load(object sender, EventArgs e)
{
if(!IsPostBack)//Only do this first time it's loaded
{
TextBoxFirstName.Text = Person.FirstName;
TextBoxLastName.Text = Person.LastName;
}
}
And your button click can look like this:
protected void ButtonSave_Click(object sender, EventArgs e)
{
SavePerson(TextBoxFirstName.Text, TextBoxLastName.Text);
}
private SavePerson(string firstName, string lastName)
{
using (PeopleEntities ctx = new PeopleEntities())
{
var person = GetPerson(ctx);
person.FirstName = firstName;
person.LastName = lastName;
ctx.SaveChanges();
}
}
As you progress with your coding you will find that you want to repeat the SavePerson
and GetPerson
code on other pages. - and that is when you start introducing repositories or layers. Don't forget the mantra principal that you should endeavour to follow and move the code to another class so that you can re-use it.
That class should be in a PeopleRepository
or some other layer. Eventually you will find that the code in the PeopleRepository
looks very like the code in the MantraRepository
and you will want to stop repeating yourself for different types.
That is when you should start using "generics". You replace the PeopleRepository
and the MantraRepository
with a Repository<People>
and a Repository<Mantra>
and the code is in one class defined something like public class BaseRepository<T>
.
Before you go on that journey though, there is another thing about the Entity Framework bit - instead of
var query = (from a in ctx.People
where a.PersonID == iPersonID
select a).FirstOrDefault();
you should/could use
var query = ctx.People.Find(iPersonID)
From this source: Querying/Finding Entities
"The Find method on DbSet uses the primary key value to attempt to find an entity tracked by the context. If the entity is not found in the context then a query will be sent to the database to find the entity there. Null is returned if the entity is not found in the context or in the database.
Find is different from using a query in two significant ways:
A round-trip to the database will only be made if the entity with the given key is not found in the context. Find will return entities that are in the Added state. That is, Find will return entities that have been added to the context but have not yet been saved to the database."
And now if you want to make that change and because you haven't repeated yourself anywhere you only have to change the code in the GetPerson method.
P.S. the code for getting a record will probably look like something like this when you finally implement that generic repository.
T e = Context.Set<T>().Find(id)
One line to get them all
Upvotes: 0
Reputation: 3147
Best approach, IMHO, is that when you're retrieving data to DISPLAY only, do it without change-tracking. This will avoid performance issues. So, use the AsNoTracking
method to avoid change-tracking proxies.
Than, for the update, you should load WITH change-tracking enabled, that's why there is no call to AsNoTracking
on the save part.
Remember to check for null values. You're using the FirstOrDefault but since you're using the primary key, there won't be a second record, so just using SingleOrDefault. But since default (null) can happen, check for null values.
Also, use lambda expressions. They are not so easy to get with at first but you'll get used with minor effort and they will simplify your code a lot.
But from your question, there are some workarounds to avoid this but they are not the best approach. You should avoid long-living entities and prefer ViewModels for long-living objects, with the UnitOfWork pattern in mind for the repository and persistent entities.
If you really want that, you can Detach
your entity from the context, use it everywhere and when you're ready, Attach
it back and set it's state to Modified
.
For this, take a look here: http://msdn.microsoft.com/en-us/library/bb896271.aspx
On your case, I'd suggest this:
private void LoadData(int iPersonID)
{
using (PeopleEntities ctx = new PeopleEntities())
{
// AsNoTracking will avoid performance hit of change-tracking here...
// Since we're building-up a view, not an update case yet, you don't have to create
// proxies that will check for entity changing...
var query = ctx.People.AsNoTracking().SingleOrDefault(_people => _people.PersonID == iPersonID)
// Rendering comes into action
if (query != null)
{
TextBoxFirstName.Text = query.FirstName;
TextBoxLastName.Text = query.LastName;
}
}
}
private void SaveEmployee(int iPersonID = 0)
{
using (PeopleEntities ctx = new PeopleEntities())
{
var query = ctx.Prople.SingleOrDefault(_person => _person.PersonID == iPersonID);
if (query != null)
{
query.FirstName = TextBoxFirstName.Text;
query.LastName = TextBoxLastName.Text;
ctx.SaveChanges();
}
}
}
Upvotes: 1
Reputation: 68
This is also how I do it. It is necessary to retrieve the object you wish to update.
Upvotes: 0