I just started working with ASP.NET MVC a few weeks ago, and I'm finding that it can be very easy to write spaghetti code in the controllers.  For my first project, I created a very simple view with a few controls.  At first, all of my code was in the Index() action result.  That worked okay for a while, but as more features get added to the page, the more the code grows.  I would like my code to be split up into multiple action results.  I made an attempt at refactoring.  Here is my View:@model TpgInternalSite.Models.RepairInvoice@{    ViewBag.Title = Repair Invoicing;    Layout = ~/Views/Shared/_Layout100.cshtml;}<h2>Repair Invoice</h2><script type=text/javascript>    $(document).ready(function () {        $('@(ViewBag.SetFocusTo)').focus();        $('#RmaNumber, #SerialNumber').keydown(function (event) {            if (event.keyCode == 13 || event.keyCode == 9) {                var RmaNumber = $('#RmaNumber').val();                var SerialNumber = $('#SerialNumber').val();                event.preventDefault(); //Stops enter key from triggering the Process button.                  if (event.target.id == 'RmaNumber'){                    var link = '/Invoice/RmaNumberScan?rmaNumber=' + RmaNumber;                    location.href = link;                }                else {                    var link = '/Invoice/SerialNumberScan?rmaNumber=' + RmaNumber + '&serialNumber=' + SerialNumber;                    location.href = link;                }            }        });    });</script><br /><br /><div class=form-horizontal>    @using (Html.BeginForm(Index, Invoice, FormMethod.Post))    {        <div class=control-group>            <div class=control-label>                RMA#            </div>            <div class=controls>                @Html.TextBoxFor(x => x.RmaNumber)            </div>        </div>        <div class=control-group>            <div class=control-label>                SERIAL#            </div>            <div class=controls>                @Html.TextBoxFor(x => x.SerialNumber)            </div>        </div>        <div class=control-group>            <div class=control-label>                Terminal Type:            </div>            <div class=controls>                @Html.LabelForModel(Model.TerminalType)            </div>        </div>        <div class=control-group>            <div class=control-label>                Warranty:            </div>            <div class=controls>               @Html.CheckBox(warranty, Model.UnderWarranty, new { disabled = disabled })            </div>        </div>        <div class=control-group>            <div class=control-label>                Repair Level:            </div>            <div class=controls>                @Html.DropDownListFor(x => x.RepairLevels, new SelectList(Model.RepairLevels))            </div>        </div>        <div class=form-actions>           <input name=process class=btn primary type=submit id=process value=Process />        </div>    }    </div></>And my controller looks like this:    public ActionResult Index()    {        ViewBag.SetFocusTo = #RmaNumber;        Session[RmaDetail] = null;        return View(repairInvoice);    }    [HttpPost]    public ActionResult Index(RepairInvoice ri)    {        if (Session[RmaDetail] != null)        {            var rmaDetail = Session[RmaDetail] as SVC05200;            RepairInvoiceDataLayer.UpdateRmaRepairLevel(rmaDetail, ri.RepairLevels[0].Trim());            ViewBag.SuccessMessage = Repair level updated successfully.;            ViewBag.SetFocusTo = #RmaNumber;            ModelState.Clear();            ri.SerialNumber = string.Empty;            Session[RmaDetail] = null;        }        return View(ri);    }    public ActionResult RmaNumberScan(string rmaNumber)    {        repairInvoice.RmaNumber = rmaNumber;        if (!string.IsNullOrEmpty(rmaNumber))        {            try            {                bool IsValidRma = RepairInvoiceDataLayer.IsValidRmaNumber(rmaNumber);                if (IsValidRma)                {                    ViewBag.SetFocusTo = #SerialNumber;                }                else                {                    ViewBag.AlertMessage = RMA Number not found.;                }            }            catch (Exception ex)            {                ViewBag.ErrorMessage = An error occured searching for RMA Number.  Please try again.;                log.Fatal(ex.ExceptionToString());            }        }        return View(Index, repairInvoice);    }  public ActionResult SerialNumberScan(string rmaNumber, string serialNumber)    {        if (!string.IsNullOrEmpty(rmaNumber) && !string.IsNullOrEmpty(serialNumber))        {            try            {                if (serialNumber.Length > 20)                {                    serialNumber = serialNumber.Remove(20);                }                ModelState[SerialNumber].Value = new ValueProviderResult(serialNumber, serialNumber, CultureInfo.CurrentCulture);                var result = RepairInvoiceDataLayer.SelectRmaDetail(rmaNumber, serialNumber);                if (result != null)                {                    if (result.Received == 1)                    {                        Nullable<bool> workOrderClosed = RepairInvoiceDataLayer.WorkOrderClosed(result.RETDOCID, result.LNSEQNBR, serialNumber);                        if (workOrderClosed.HasValue)                        {                            if (workOrderClosed.Value)                            {                                Session[RmaDetail] = result;                                repairInvoice.TerminalType = result.ITEMNMBR.Trim();                                repairInvoice.UnderWarranty = RepairInvoiceDataLayer.UnderWarranty(result.RETDOCID, result.LNSEQNBR, serialNumber, result.CUSTNMBR);                                ViewBag.SetFocusTo = #RepairLevels;                            }                            else                            {                                ViewBag.AlertMessage = Work order not closed.;                            }                        }                        else                        {                            ViewBag.AlertMessage = Work order not found.;                        }                    }                    else                    {                        ViewBag.AlertMessage = Line item has not been received.;                    }                }                else                {                    ViewBag.AlertMessage = Serial Number not found.;                }            }            catch (Exception ex)            {                ViewBag.ErrorMessage = An error occured searching for Serial Number.  Please try again.;                log.Fatal(string.Format(An error occured searching for Serial Number.  RMA Number: {0}  Serial Number: {1}, rmaNumber, serialNumber, ex));            }        }        return View(Index, repairInvoice);    }Basically my page is setup to handle this current process flow:User types a number into the RMA number textbox and hits enter or tab.  JavaScript detects enter or tab key and navigates to /Invoice/RmaNumberScan and passes a query string with the number entered.  Logic is performed in the RmaNumberScan action result.  Next, the user types a number into the serial scan textbox and hits tab or enter.  The /Invoice/SerialNumberScan is called and passes query strings to the SerialNumberScan method.  User clicks the process button and then the Index() with the HTTPPOST method fires.Am I on the right track, or am I doing MVC incorrectly? I come from a webforms background, and I want to split up my code.  My attempt just feels so dirty and I don't like the fact that my action result name appears in the URL.  I just want my code to be easily maintained and readable.Since all of my database logic is done in my RepairInvoiceDataLayer, I still don't understand how the service layer comes into play here.  What code from this controller would go into the service layer?
ASP.NET MVC to produce a repair invoice
c#;asp.net mvc 4
It sounds like you need to add a service layer where you can include all your business logic. This way your controller classes do not become bloated with business logic and they are only dealing with the handling of requests and the population of view-models.Using thin controllers like this you can separate your logic out to different services and make it easier to practice the Single Responsibility Principle.Please see this question and accepted answer but I've included an example below: Creating a service layer for my MVC applicationYour controller:[Dependency]public IInvoiceService InvoiceService;public ActionResult Index(){    //code    return View(repairInvoice);}[HttpPost]public ActionResult Index(RepairInvoice ri){     this.InvoiceService.ProcessInvoice(ri.RmaNumber, ri.SerialNumber); // add on other params as appropriate    return View(ri);}Service layer would consist of multiple classes, each with their own responsibility such as:public class InvoiceService : IInvoiceService{    public void ProcessInvoice(string rmaNumber, string serialNumber)     {        // Do any processing here    }}I would recommend creating an interface for each separate service and then you can use Dependency Injection to instantiate the concrete classes using Unity or similar.