How could I make my controller a little less hideous? I have a controller. It functions as follows:

A user imports a file and decides whether the files contents contain new entries to be placed into a database, or existing entries entries to be updated.

The user clicks on the import button and is sent to the controller below. If there are any warnings or errors, the validation results screen is shown - to help the users correct their data.

If the file doesn't contain any errors or warnings, the validation screen will be skipped and their file contents will be inserted/updated into the database.

This is clearly far too much logic for a controller as I have read that the controller is simply meant to decide which view to send the data to. I am having a rather difficult time determining how to extract out the data.

I am looking for suggestions on what my next step should be. Thank you to anyone who takes time to help me.

public ActionResult ValidationResultsScreen(HttpPostedFileBase file, DatabaseAccessMode databaseAccess)
{
    try
    {
        var fileLocation = Path.Combine(Path.GetTempPath(), string.Join(Guid.NewGuid().ToString(), file.FileName));

        Session["TempFileLocation"] = fileLocation;
        Session["DatabaseAccessMode"] = databaseAccess;

        file.SaveAs(fileLocation);

        var importer= new Importer(_connectionString, _logManager, databaseAccess,Server.MapPath("..\\bin"));

        var parsedValues= importer.ParseFile(fileLocation);

        var failedValues = ParsedValues.Where(x =&gt; x.ParsedState != ParsedState.Success);

        if (failedValues.Any())//if there were any warnings or errors display the validation results screen
        {
            return View(processedValues);
        }
        //else import the file to the database

        var transactionResults = importer.UploadToDatabase(parsedValues);

        var failedDBTransactions= RetrieveFailedResults(transactionResults);

        return failedDBTransactions.Any() 
            ? View("DatabaseImportErrorScreen", failedDBTransactions) 
            : View("DatabaseImportResults", transactionResults);
    }
    catch (Exception exception)
    {
        ViewBag.ExceptionMessage = exception.Message;
        return View("DefaultErrorScreen");
    }
}
Based on your case your controller seems to be mixing multiple things at the same time. The total context is not clear but it seems this is a controller method in a controller that does more than this, correct?

In general I would suggest to see it different. An import is an object, so create a seperate controller for it: ImportsController. That can be a very standard controller:

Add: User add a new file, controller just sends it to Import model -> save. The model saves it in a temp file or whatever it needs to do. You can implement validation here already but you can also say: "A upload and saved file is ok. That is a correct creation of an import."

Pseudo code:

if(dataIncoming) {
    this-&gt;Import-&gt;save(data)
    //handle file save errors, no data received etc. All coming back from model which should take care of the validation.
    //redirect to edit
}


Edit: Next step is to review the import. Here you can ask the model to validate.

Pseudo code:

ON SAVE (if this is requested without a save/submit action just show data and validation results)
this-&gt;Import-&gt;parseFile(data) //validate

//show parse file issues to user if any
return View(processedValues);

if(no errors in parsefile) {
  //try database save
  Import-&gt;saveInDatabase
  //show validation errors if needed

  //if all ok you can redirect (or have user interaction) to delete
}


View: Always shows you the data of the import

Delete: Can be used to remove a wrong import upload (user uploaded file twice) or when import is done. You can validate this.

Here you can implement also a soft delete for example.

Pseudo code:

Import-&gt;delete


So as you see I would move all that file, tmpfile and other parts to the model by seeing an import as a standalone object. That makes it possible to have all encapsulated and also well testable. A controller method doing all steps is much harder to test. For example you would need to catch all different views rendered etc. I prefer to split it up.