Reputation: 7818
Following some excellent help from this forum, can anyone please confirm if this is the best way to update data in a database, in an MVC Controller, where multiple records/models are posted back at once?
[HttpPost]
public ActionResult Edit(ObjectivesEdit model)
{
if (model.Objectives != null)
{
// model will have several records posted back - so loop through each one, and update the database
foreach (var item in model.Objectives)
{
// find the database row
Objective objective = db.objectives.Find(item.ID);
// Set the database row to the posted values
objective.objective = item.objective;
objective.score = item.score;
objective.possscore = item.possscore;
objective.comments = item.comments;
}
// Save the changes to the database
db.SaveChanges();
}
return View(model);
}
The part I think probably has a better way of working is:
objective.objective = item.objective;
objective.score = item.score;
objective.possscore = item.possscore;
objective.comments = item.comments;
Can that be replaced with something more streamlined, or is that the way to do this?
Upvotes: 0
Views: 2982
Reputation: 1125
The biggest issue i see with your code is that you could potentially hit the database for every record that is returned. Also if your holding the entire object on the form the mapping isn't completely necessary
So you would want to query all necessary objectcs in one query
var Objectives = db.Objectives.where(o => model.Objectives.select(ob => ob.ID).Contains(o.ID));
this will bring back all the Objectives from the database that have been posted to the controller action.
then inside your foreach change
Objective objective = db.objectives.Find(item.ID);
to
Objective objective = Objectives.First(o => o.ID == item.ID);
Then continue your mapping
Just always try and minimize your trips to the database
Upvotes: 0
Reputation: 777
It is better to use repository pattern for updating or adding to database.
look at the link below
https://codereview.stackexchange.com/questions/6266/asp-net-mvc-using-repository-pattern-code-review
Upvotes: 1
Reputation: 1492
Other than using lambda expressions such as:
var employees = data.Employees.Where(em => em.City.StartsWith("B")).ToList();
employees.ForEach(em => em.City = "ABC");
(above taken from here)
You would have to update records in a loop as you are currently doing it. Note that the above is still a loop, but is executed internally (and looks a bit cleaner).
Upvotes: 0