inserting collection of objects into datastore from ASP.NET MVC controller

When you have a number of items to insert into DB from a ASP.NET MVC app (like a collection of posted files) do you perform most of the exception handling logic in the controller or inisde the repository?

Do you do this

    [AcceptVerbs(HttpVerbs.Post)]
    public virtual ActionResult Import(HttpPostedFileBase fileToUpload)
    {
        string path = Server.MapPath("~/App_Data/Upload/") + Path.GetFileName(fileToUpload.FileName);
        fileToUpload.SaveAs(path);
        string selectCmd = "SELECT * FROM [Pricelist$]";

        try {
            foreach (DataRow dr in ExcelUtility.ReadFile(path, selectCmd).Tables[0].Rows) {
                Product p = new Product
                {
                    Title = dr["product_name1"].ToString().Trim(),
                 开发者_JAVA百科   Measure = dr["product_measure"].ToString().Trim(),
                };

                if (dr["product_price"] != DBNull.Value)
                    p.Price = (double)dr["product_price"];
                if (dr["product_taxrate"] != DBNull.Value)
                    p.TaxRate = (double)dr["product_taxrate"];
                _repository.Add(p);
            }
        }
        catch (Exception e) {
            // handle exception or run the action in transaction??
        }

        return RedirectToAction("ImportSuccessful");
    }

or something else (maybe handle the exception in the repository and have repository action return true/false based on whether the method was successfully executed or not)?

SQL query and DataRows inside a controller action? Oh no. This code reminds me the good 'ol days of WebForms :-) You should really be putting your controllers on a diet.

As @Hector and @Darin Dimitrov said, you should make the controller thin.

To answer your question "handle exception or run the action in transaction?",

• Yes, you need to run in the transaction if your application doesn't allow to import partially. You may want to use Unit Of Work ActionFilter.
• Handle exception? I would catch SomeSpecificException which may be raised by ExcelUtility.ReadFile method but not the Exception class. And you need to define what are the exceptional cases according to you application requirement. Here is the great blog post about exception handling by Scott Hanselman.

I would start by moving all the logic that you have inside the try block into a a model (or a repository). That would leave very little in the controller -- and that's a good thing.

Perhaps the only logic that I would leave in the controller would be an if block to decide whether the user needs to be redirected to a "success" or "error" page.

Since my question wasn't formulated to the best of my abilities because I didn't actually know what exactly I would like for the answer, I'm going to post the solution that I've came up with in this last day.

First, my repository handles all of the data functions. Second, my repository method return true/false whether the data insertions were successful (there may be problems with inserting into database that are not exceptions, just some business rules failing). Third, my repository method builds a report that is returned via output params.

This is the controller action code now:

    //
    // POST: /admin/post/import
    [HttpPost]
    public virtual ActionResult Import(HttpPostedFileBase fileToUpload)
    {
        string path = Server.MapPath("~/App_Data/Upload/") + Path.GetFileNameWithoutExtension(fileToUpload.FileName) + "-" + Guid.NewGuid() + Path.GetExtension(fileToUpload.FileName);
        fileToUpload.SaveAs(path);
        string report = string.Empty;
        bool success = _repository.ImportPostcodes(path, out report);
        TempData["ImportReport"] = report;
        if (success)
            return RedirectToAction(MVC.admin.Post.ActionNames.ImportSuccess);
        else
            return RedirectToAction(MVC.admin.Post.ActionNames.ImportFailed);
    }

And this is the repository interface:

public interface IPostCodeRepository
{
    IApplicationState AppState { get; set; }
    void Add(PostCode instance);
    void Delete(PostCode instance);
    void Delete(Guid rowGuid);
    void Delete(string postalCode);
    IQueryable<PostCode> GetAll();
    IQueryable<PostCode> GetAll(string query);
    PostCode Load(Guid rowGuid);
    PostCode Load(string postalCode);
    void SaveChanges();
    void Update(PostCode instance);
    bool ImportPostcodes(string path, out string report);
}

I think this is as optimal as it gets..but if someone has a comment on it, be my guest.