Best practice checking asp.net mpc controller

Question

I'm a little confused about the "best practice" controller using the question.

My typical code view

public ActionResult Edit(int reportId,FormCollection formCollection) { try { var report = _dbContext.EmployeeReports.Find(reportId); if (TryUpdateModel(report)) { _employeeReportService.Update(report); return RedirectToAction("List"); } return View("Edit", report); } catch (Exception) { // some logging etc return RedirectToAction("List"); } 

Well, is it better to use "TryUpdateModel" or just "UpdateModel" or just call Model.IsValid and is there a good idea to catch the exception in the controller?

thanks

Answer 1

It depends if you expect and plan to deal with exceptions.

My usual approach:

 public ActionResult foobar(FormCollection formCollection) { //Keep this out of the try catch scope in case you need to pass it // to the next method. Model model = new Model(); try { if(!TryUpdateModel(model) { //Update Failed so fix it and redirect return redirectToAction("fixit"); } if(!ModelState.IsValid()) { //Update worked but model state was invalid, return to page to correct //model validation errors return View("foobar", model); } //Update Succeeded so do other stuff } catch(Exception ex) { //Deal with Exception return redirectToAction("ErrorView", "ErrorController"); } return redirectToAction("NextStep"); } 

I try to use all of them in my code to try to catch every problem before it breaks something.

Answer 2

Here is an alternative way that I prefer:

 [HttpPost] public ActionResult Edit(ReportViewModel reportViewModel) { if (!ModelState.IsValid) { // there were validation errors => redisplay the form // so that the user can fix them return View(reportViewModel); } // At this stage the view model is valid => we can // map it back to a domain model and pass to the repository // for processing // Fetch the domain model that we want to update var report = _repository.Get(reportViewModel.Id); // map the domain model properties from the view model properties // in this example I use AutoMapper Mapper.Map<ReportViewModel, Report>(reportViewModel, report); // perform update _repository.Update(report); // the update wen fine => we can redirect back to the list action return RedirectToAction("List"); } 

So, since you do not see FormCollection , no TryUpdateModel , no UpdateModel , no try/catch .

Answer 3

In my opinion, you should always use view models instead of formcollection to avoid under-deliveries and forwarding problems. Therefore, in my opinion, the best practice is to use the view model to render the view and the type of post / get model that is related to what you want users to be able to post / receive from the action.

It might be some kind of extra work, and some of the view models will be very similar to the models you use to bind to the action of the controller, but I would say "Security versus convenience."