4
\$\begingroup\$

I've written the following controller and i was looking for some input/advice on how i could go about cleaning this up or rewriting it. I feel like i'm repeating myself a lot or that there might be a better way to do this.

namespace WebUI.Controllers
{
    [Authorize(Roles="Administrator,Registrar")]
    public class RegistrarController : BaseController
    {
        private readonly IRegistrationService _registrationService;

        public const string WizardKey = "Wizard";
        public const string ActionKey = "Action";

        public RegistrarController(IRegistrationService registrationService)
        {
            this._registrationService = registrationService;
        }



        [HttpGet]
        public ActionResult Step1()
        {
            if (string.IsNullOrEmpty(this.RegistrarId))
            {
                ViewBag.ErrorMessage = "A valid registrar id is not associated with this account.";
                return View("Error");
            }
            PersonRegistrationWizard wizard;
            if (!PrepareAndCheckStep(1, out wizard)) { /* Should always be true... */ }

            return View(wizard.Step1Model);
        }

        [HttpPost]
        public ActionResult Step1(NewRegistrantModel model) 
        {
            var wizard = TempData[WizardKey] as PersonRegistrationWizard;
            if (wizard == null)
                wizard = new PersonRegistrationWizard();

            if (!ModelState.IsValid)
            {
                wizard.Step1Model = model;
                TempData[WizardKey] = wizard;
                return View("Step1", model);
            }

            var eventDate = _registrationService.GetUpcomingEventFor(DateTime.Now);


            if (!eventDate.HasValue)
            {
                wizard.Step1Model = model;
                TempData[WizardKey] = wizard;
                ViewBag.ErrorMessage = "Sorry, no upcoming event was found within our database.";
                return View("Error");
            }

            if (!_registrationService.EligibleToRegister(model.Birthdate.Value, eventDate.Value))
            {
                wizard.Step1Model = model;
                TempData[WizardKey] = wizard;
                ModelState.AddModelError("","Error Goes Here");
                return View("Step1", model);
            }


            wizard.Step1Model = model;
            if (wizard.MaxCompletedStep < 1)
                wizard.MaxCompletedStep = 1;

            TempData[WizardKey] = wizard;

            return RedirectToAction("Step2");
        }


        [HttpGet]
        public ActionResult Step2()
        {
            PersonRegistrationWizard wizard;

            if (!PrepareAndCheckStep(2, out wizard))
            {
                return RedirectToAction("Step1");
            }
            return View(wizard.Step2Model);


        }


        [HttpPost]
        public ActionResult Step2(PersonInformationModel model) 
        {

            var wizard = TempData[WizardKey] as PersonRegistrationWizard;
            if (wizard == null || wizard.Step1Model == null || wizard.MaxCompletedStep < 1)
                return RedirectToAction("Step1");

            if (!ModelState.IsValid)
            {
                wizard.Step2Model = model;
                TempData[WizardKey] = wizard;
                return View("Step2", model);
            }

            model.Person.NotInStreetIndex = !StreetDataValid(model.Person);

            if (model.Person.NotInStreet && !model.ConfirmResident)
            {
                wizard.Step2Model = model;
                TempData[WizardKey] = wizard;
                 ModelState.AddModelError("ConfirmResident", "The address entered was not found in our database. Please confirm that you a resident of ....");
                return View(model);
            }

            wizard.Step2Model = model;
            wizard = MoveDataFromStep1ToStep2(wizard);

            if (wizard.MaxCompletedStep < 2) 
                wizard.MaxCompletedStep = 2;

            TempData[WizardKey] = wizard;

            return RedirectToAction("Step3");
        }




        [HttpGet]
        public ActionResult Step3()
        {
            PersonRegistrationWizard wizard;

            if (!PrepareAndCheckStep(3, out wizard))
                return RedirectToAction("Step1");

            return View(wizard);
        }


        [HttpPost]
        public ActionResult Step3(FormCollection form)
        {
            var wizard = TempData[WizardKey] as PersonRegistrationWizard;
            if (wizard == null ||  wizard.MaxCompletedStep < 2 || wizard.Step1Model == null || wizard.Step2Model == null)
                return RedirectToAction("Step1");

            if (!string.IsNullOrEmpty(form["editStep1"]))
                return RedirectToAction("Step1");
            else if (!string.IsNullOrEmpty(form["editStep2"]))
                return RedirectToAction("Step2");
            else
            {
                if (wizard.MaxCompletedStep < 2)
                {
                   // Display Error
                }
                if (ModelState.IsValid)
                {

                    wizard = MoveDataFromStep1ToStep2(wizard);

                    var Person = BuildPersonFromModel(wizard);
                    var registration = new RegistrarPersonRegistration();

                    if (Person.Id > 0)
                    {
                        registration = _registrationService.GetManyRegistrarPersonRegistrationsBy(v => v.PersonId == Person.Id).FirstOrDefault();

                        if (registration == null || registration.Person == null)
                            throw new InvalidProgramException("The Person id supplied does not have a Person registration");

                        Person.LastUpdatedOn = DateTime.Now;
                        registration.Person.Copy(Person);
                        registration = _registrationService.UpdateRegistrarPersonRegistration(registration);

                    }
                    else
                    {

                        Person.CreatedBy = this.User.Identity.Name;
                        Person.CreatedOn = DateTime.Now;
                        Person.RegistrationTypeCode = "REG";
                        Person.RegistrationStatusCode = "P";
                        Person.RegistrationDate = DateTime.Now;

                        registration = _registrationService.RegistrarRegisterPerson(wizard.RegistrarId, Person);

                    }

                    _registrationService.Save();
                    wizard.Step2Model.Person.PersonId = registration.PersonId;
                    if (wizard.MaxCompletedStep < 3)
                        wizard.MaxCompletedStep = 3;

                    TempData[WizardKey] = wizard;
                }
                else
                {

                    // TO DO: Display Errors that prevent the user from continuing
                    TempData[WizardKey] = wizard;
                    return View(wizard);
                }
                return RedirectToAction("Step4");
            }

        }


        [HttpGet]
        public ActionResult Step4()
        {
            PersonRegistrationWizard wizard;
            if (!PrepareAndCheckStep(4, out wizard))
                return RedirectToAction("Step1");


            return View(wizard);
        }


        [HttpPost]
        public ActionResult Step4(FormCollection form)
        {
            var wizard = TempData[WizardKey] as PersonRegistrationWizard;
            string action = form[ActionKey] == null ? string.Empty : form[ActionKey].ToString().ToUpper();

            if (wizard == null || wizard.MaxCompletedStep < 3 || 
            wizard.Step2Model == null || 
            wizard.Step2Model.Person == null || 
            !wizard.Step2Model.Person.PersonId.HasValue || 
            wizard.Step2Model.Person.PersonId.Value <= 0)
                return RedirectToAction("Step1");

            int PersonId = wizard.Step2Model.Person.PersonId.Value;
            var registration = new RegistrarPersonRegistration();

            switch (action)
            {
                case "PRINT":
                    registration = _registrationService.GetManyRegistrarPersonRegistrationsBy(rvr => rvr.PersonId == PersonId).FirstOrDefault();
                    if (registration == null)
                    {
                        ViewBag.ErrorMessage = "No Person id was provided.";
                        wizard = null;
                        return View("Error");
                    }

                     registration.Person.RegistrationStatusCode = "C";
                    _registrationService.UpdateRegistrarPersonRegistration(registration);
                    _registrationService.Save();
                    TempData[WizardKey] = null;
                    // Return Form
                    return RedirectToAction("GenerateRegistrationForm", "Report", new { id = registration.Id });
                    break;
                case "ADD":
                    TempData[WizardKey] = null;
                    return RedirectToAction("Step1");
                case "HOME":
                    TempData[WizardKey] = null;
                    return RedirectToAction("Dashboard");
                default:
                    break;
            }

            return View(wizard);
        }


        /// <summary>
        /// Prepares the wizard and checks to see if the previous steps were completed
        /// </summary>
        /// <param name="step">Current step</param>
        /// <param name="wizard">Registration Wizard</param>
        /// <returns>returns true if the previous steps have been completed and false if not</returns>
        private bool PrepareAndCheckStep(int step, out PersonRegistrationWizard wizard)
        {
            bool isValid = false;

            wizard = TempData[WizardKey] as PersonRegistrationWizard;

            switch (step)
            {
                case 1:
                    if (wizard == null || wizard.Step1Model == null)
                    {
                        wizard = new PersonRegistrationWizard();
                        wizard.RegistrarId = this.RegistrarId;
                        wizard.Step1Model = new NewRegistrantModel();
                    }
                    isValid = true;
                    break;
                case 2:
                    if (wizard != null && wizard.MaxCompletedStep >= 1 && wizard.Step1Model != null)
                    {
                        isValid = true;
                        if (wizard.Step2Model == null)
                            wizard.Step2Model = new PersonInformationModel();
                        wizard = MoveDataFromStep1ToStep2(wizard);
                    }
                    break;
                case 3:
                case 4:
                    isValid = wizard != null && wizard.MaxCompletedStep >= step - 1 && wizard.Step1Model != null && wizard.Step2Model != null;
                    break;
                default:
                    if (wizard == null)
                        wizard = new PersonRegistrationWizard();
                    break;
            }


            if (wizard != null)
            {
                if (string.IsNullOrEmpty(wizard.RegistrarId))
                    wizard.RegistrarId = this.RegistrarId;

                wizard.CurrentStep = step;
                TempData[WizardKey] = wizard;
            }

            return isValid;
        }


        private Person BuildPersonFromModel(PersonRegistrationWizard wizard)
        {
            int PersonId = 0;
            var modelPerson = wizard.Step2Model.Person;

            var Person = new Person();
            if (modelPerson.PersonId.HasValue)
            {
                Person = _registrationService.GetPersonBy(v => v.Id == modelPerson.PersonId.Value);
                if (Person == null)
                    throw new InvalidOperationException(string.Format("An invalid Person id was supplied {0}", Person.Id));
                PersonId = Person.Id;
            }

            Person = modelPerson.ToEntity();
            Person.Id = PersonId;

            return Person;
        }

        private PersonRegistrationWizard MoveDataFromStep1ToStep2(PersonRegistrationWizard wizard)
        {
            if (wizard == null || wizard.Step1Model == null)
                return null;
            if (wizard.Step2Model == null)
            {
                wizard.Step2Model = new PersonInformationModel();
            }

            wizard.Step2Model.Person.Identification = wizard.Step1Model.Identification;
            wizard.Step2Model.Person.Birthdate = wizard.Step1Model.Birthdate;
            wizard.Step2Model.Person.SocialSecurityNumber = wizard.Step1Model.SocialSecurityNumber;
            wizard.Step2Model.Person.IdSupplied = wizard.Step1Model.OtherIdProvided;
            return wizard;
        }

        private bool StreetDataValid(PersonData Person)
        {
            return _registrationService.ValidateStreet(Person.AddressNumber.Value, Person.AddressDirection, Person.AddressStreet, Person.AddressStreetType, Person.AddressPostDirection, Person.AddressCity, Person.AddressZip);

        }



    }
}

Thanks

\$\endgroup\$
3
\$\begingroup\$

Here the UI model is tightly coupled with the data structures required. Hence the need arises for transfering data from step 1 to step 2 etc. Instead the data structures should be based on business objects required - e.g. Person object with nested objects like social security info, personal info, residence info etc. Let all steps deal with the same Person object, updating inputs given by user in each step.

Also the 'Step' itself can be separated out into an interface / abstract class. Let's say IStep is in interface with ValidateInput(), Prepare() and ExecuteStep() methods. ExecuteStep() internally calls ValidateInput() and Prepare() and returns ActionResult object. Then we have 4 small subclasses, one for each step implementing IStep. The controller code will be simplified, it just needs to create an instance of next step (depending on ActionResult received from earlier step) and call Execute() on the same. All step objects will share the same Person object, which can be passed to the constructor.

Please reply back with specific queries if this approach is not clear.

Following are the advantages of this approach

  1. The controller code is much simpler
  2. It is easier to modify a step
  3. It is easier to add/remove a step. Controller class will have very minimal effect of calling the new step.Execute() or removing call for an existing step. Other steps will not be affected at all.
\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the feedback! I had a question though. If i were to use a business object instead of the data structure how would i go about using the built in validation with data annotations that asp.net mvc framework provides? Some properties of the business object would not required until a certain step is complete. The reason i had to seperate it was because of this. Thanks once again! \$\endgroup\$
    – zSysop
    Sep 28 '11 at 14:04

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.