6
\$\begingroup\$

Basically, I'm uploading an excel file and parsing the information then displaying what was parsed in a view.

using System.Data;
using System.Data.OleDb;
using System.Web;
using System.Web.Mvc;
using QuimizaReportes.Models;
using System.Collections.Generic;
using System;

namespace QuimizaReportes.Controllers
{
    public class UploadController : Controller
    {
        public ActionResult Index()
        {
            return View();
        }

        [HttpPost]
        public ActionResult Index(HttpPostedFileBase excelFile)
        {
            if (excelFile != null)
            {
                //Save the uploaded file to the disc.
                string savedFileName = "~/UploadedExcelDocuments/" + excelFile.FileName;
                excelFile.SaveAs(Server.MapPath(savedFileName));

                //Create a connection string to access the Excel file using the ACE provider.
                //This is for Excel 2007. 2003 uses an older driver.
                var connectionString = string.Format("Provider=Microsoft.ACE.OLEDB.12.0;Data Source={0};Extended Properties=Excel 12.0;", Server.MapPath(savedFileName));

                //Fill the dataset with information from the Hoja1 worksheet.
                var adapter = new OleDbDataAdapter("SELECT * FROM [Hoja1$]", connectionString);
                var ds = new DataSet();
                adapter.Fill(ds, "results");
                DataTable data = ds.Tables["results"];

                var people = new List<Person>();

                for (int i = 0; i < data.Rows.Count - 1; i++)
                {
                    Person newPerson = new Person();
                    newPerson.Id = data.Rows[i].Field<double?>("Id");
                    newPerson.Name = data.Rows[i].Field<string>("Name");
                    newPerson.LastName = data.Rows[i].Field<string>("LastName");
                    newPerson.DateOfBirth = data.Rows[i].Field<DateTime?>("DateOfBirth");

                    people.Add(newPerson);
                }

                return View("UploadComplete", people);
            }

            return RedirectToAction("Error", "Upload");                      
        }

        public ActionResult Error()
        {
            return View();
        }
    }
}

Not feeling so confident this is the best approach. Any suggestion any of you MVC3 vets have for this aspiring senior programmer? :)

\$\endgroup\$
1
  • \$\begingroup\$ Should I call another Action, "UploadComplete" and have that callthe UploadComplete view instead of calling a View directly from the [POST]Index action? When do I know whether to use one approach or the other? \$\endgroup\$
    – Sergio Tapia
    Feb 25 '11 at 2:26
10
\$\begingroup\$

The logic for reading the Excel file (i.e. everything from var connectionString until before the return) belongs in a model method, not the controller.

You might also want to handle the case when the uploaded file isn't an Excel file or it doesn't have the columns you expect it to. Depending on how you use this or who uses it, this might not be necessary, but if normal users are allowed to upload files, they might very well upload the wrong file even if you tell them not to. And in that case a meaningful error message leads to a better user experience.

Regarding the question in your comment: As I said, I don't know how this is going to be used, but I can't think of a situation where it would be helpful for the user to be able to re-upload the file by hitting the refresh button. So yes, redirecting to an UploadComplete action makes sense.

\$\endgroup\$
8
\$\begingroup\$

What I like to do is create Service classes for my controllers. For your example, I'd do something like this:

UploadControllerService

public class UploadControllerService
{
    public IList<Person> GetPeopleFromFile(HttpPostedFileBase excelFile, HttpServerUtilityBase server)
    {
        if (excelFile == null)
        {
            return null;
        }

        string savedFileName = SaveFileToDisk(excelFile, server);

        // Create a connection string to access the Excel file using the ACE provider.
        // This is for Excel 2007. 2003 uses an older driver.
        string connectionString = CreateConnectionString(server, savedFileName);

        var people = GetPeopleDataFromFile(connectionString);

        return people;
    }

    private static string SaveFileToDisk(HttpPostedFileBase excelFile, HttpServerUtilityBase server)
    {
        string savedFileName = "~/UploadedExcelDocuments/" + excelFile.FileName;

        excelFile.SaveAs(server.MapPath(savedFileName));

        return savedFileName;
    }

    private static string CreateConnectionString(HttpServerUtilityBase server, string savedFileName)
    {
        return string.Format("Provider=Microsoft.ACE.OLEDB.12.0;Data Source={0};Extended Properties=Excel 12.0;", server.MapPath(savedFileName));
    }

    private static IList<Person> GetPeopleDataFromFile(string connectionString)
    {
        DataSet ds = new DataSet();

        using (var adapter = new OleDbDataAdapter("SELECT * FROM [Hoja1$]", connectionString))
        {
            adapter.Fill(ds, "results");
        }

        DataTable data = ds.Tables["results"];

        var people = new List<Person>();

        for (int i = 0; i < data.Rows.Count - 1; i++)
        {
            Person newPerson = new Person
            {
                Id = data.Rows[i].Field<double?>("Id"),
                Name = data.Rows[i].Field<string>("Name"),
                LastName = data.Rows[i].Field<string>("LastName"),
                DateOfBirth = data.Rows[i].Field<DateTime?>("DateOfBirth")
            };

            people.Add(newPerson);
        }

        return people;
    }
}

Upload Controller

public partial class UploadController : Controller
{
    private readonly UploadControllerService uploadControllerService;

    public UploadController(UploadControllerService uploadControllerService)
    {
        this.uploadControllerService = uploadControllerService;
    }

    public UploadController()
    {
        // Or this if you're not using Dependency Injection.
        uploadControllerService = new UploadControllerService();
    }

    [HttpPost]
    public virtual ActionResult Index(HttpPostedFileBase excelFile)
    {
        var people = this.HomeControllerService.GetPeopleFromFile(excelFile, this.Server);

        if (people == null)
        {
            return this.View("Error", "Upload");
        }

        return View("UploadComplete", people);
    }
}

I like to keep my controllers as lean as possible, and using Service classes has helped me.

\$\endgroup\$
2
\$\begingroup\$

I would look at seperating the different modules out much like Jason and sepp2k have already mentioned. Your controller action seems to have way too much different knowledge so I would try and seperate the responsibilities out. I like to think of the controller as having no knowledge on how something is done but knows what to use in order to get it done.

  1. Data access for reading excel file, containing the connection string details etc
  2. Logic for reading from a data source and building your model
  3. Logic for taking your model and putting it in a format for supplying back to the view (only really if your using viewmodels)

Each of these would be provide a different service and so you should be able to provide specific unit tests for each different functionality provided.

The controller then just needs to put all the pieces together.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy