Selecting security questions and entering answers

I have a dialog that houses 6 security questions and 6 security question answers.

I want to pass the data the user selects and enters from the view to the controller to save the data in session. Right now, the code works perfectly but it is a mess. I am passing 12 arguments to my controller action! I am validating the answers and passing the data to session on submit.

Need help on refactoring and simplification.

Here’s my view code:

var RequestSecurityQuestions_Submit = function () {

    ValidationAttribute.BlankValue(true);
    var form = $('form#RequestSecurityQuestions');
    $.validator.unobtrusive.parse(form);

    var d = $('form#RequestSecurityQuestions').serialize();
    SecurityQuestionsValid = true;

    var inputs = $('form#RequestSecurityQuestions').find('input[data-val]');
    $.each(inputs, function (index) {
        var input = inputs[index];
        if (!$(input).valid()) {
            SecurityQuestionsValid = false;
        }
    });

    var dropdowns = $("input.customdropdownlist");
    var dd1Value = $(dropdowns[0]).data("kendoDropDownList").value();
    var dd2Value = $(dropdowns[1]).data("kendoDropDownList").value();
    var dd3Value = $(dropdowns[2]).data("kendoDropDownList").value();
    var dd4Value = $(dropdowns[3]).data("kendoDropDownList").value();
    var dd5Value = $(dropdowns[4]).data("kendoDropDownList").value();
    var dd6Value = $(dropdowns[5]).data("kendoDropDownList").value();

    var answer1 = $('#idAnswer1').val();
    var answer2 = $('#idAnswer2').val();
    var answer3 = $('#idAnswer3').val();
    var answer4 = $('#idAnswer4').val();
    var answer5 = $('#idAnswer5').val();
    var answer6 = $('#idAnswer6').val();

    if (SecurityQuestionsValid) {
        $.ajax({
            url: Url.getFullUrl('Account/RequestSecurityQuestions_Submit'),
            type: 'Post',
            data: {AnswerOne: answer1, AnswerTwo: answer2, AnswerThree: answer3, AnswerFour: answer4, AnswerFive: answer5, AnswerSix: answer6, SecurityQuestionOne: dd1Value, SecurityQuestionTwo: dd2Value, SecurityQuestionThree: dd3Value, SecurityQuestionFour: dd4Value, SecurityQuestionFive: dd5Value, SecurityQuestionSix: dd6Value},
            cache: false,
            success: function (data) {
                PopUpSupport.ClosePopup("RequestSecurityQuestions");
            },
            error: AjaxLog.HandleAjaxCallFail
        });
    }
    return SecurityQuestionsValid;
}

Here’s my controller:

[AllowAnonymous]
    [HttpPost]
    public ActionResult RequestSecurityQuestions_Submit(string AnswerOne, string AnswerTwo, string AnswerThree, string AnswerFour, string AnswerFive, string AnswerSix, string SecurityQuestionOne, string SecurityQuestionTwo, string SecurityQuestionThree, string SecurityQuestionFour, string SecurityQuestionFive, string SecurityQuestionSix)
    {
        SecurityQuestions securityQuestions = new SecurityQuestions();

         if (string.IsNullOrEmpty(AnswerOne))
        {
            securityQuestions.ChallengeA1 = AnswerOne;
        }
        if (string.IsNullOrEmpty(AnswerTwo))
        {
            securityQuestions.ChallengeA2 = AnswerTwo;
        }
        if (string.IsNullOrEmpty(AnswerThree))
        {
            securityQuestions.ChallengeA3 = AnswerThree; 
        }
        if (string.IsNullOrEmpty(AnswerFour))
        {
            securityQuestions.ChallengeA4= AnswerFour;
        }
        if (string.IsNullOrEmpty(AnswerFive))
        {
            securityQuestions.ChallengeA5= AnswerFive;
        }
        if (string.IsNullOrEmpty(AnswerSix))
        {
            securityQuestions.ChallengeA6 = AnswerSix;
        }

        if (string.IsNullOrEmpty(SecurityQuestionOne))
        { 
        securityQuestions.ChallengeQ1Id = Int32.Parse(SecurityQuestionOne);
        }
        if (string.IsNullOrEmpty(SecurityQuestionTwo)) 
        {
            securityQuestions.ChallengeQ2Id = Int32.Parse(SecurityQuestionTwo);
        }
        if (string.IsNullOrEmpty(SecurityQuestionThree))
        {
            securityQuestions.ChallengeQ3Id = Int32.Parse(SecurityQuestionThree);
        }
        if (string.IsNullOrEmpty(SecurityQuestionFour))
        {
            securityQuestions.ChallengeQ4Id = Int32.Parse(SecurityQuestionFour);
        }
        if (string.IsNullOrEmpty(SecurityQuestionFive))
        {
            securityQuestions.ChallengeQ5Id = Int32.Parse(SecurityQuestionFive);
        }
        if (string.IsNullOrEmpty(SecurityQuestionSix))
        {
            securityQuestions.ChallengeQ6Id = Int32.Parse(SecurityQuestionSix);
        }
        UserSession.AddValue(StateName.CreateOrEditAccount, "CurrentSecurityQuestions", securityQuestions); 
        JsonResult res = Json(new { Success = true, data = "", Message = "" });
        return res;
    }

Variable names like AnswerOne are easier to work with if they that the form answer1 instead. You can then loop over them by incrementing an integer.

For example where you have

var dropdowns = $("input.customdropdownlist");
var dd1Value = $(dropdowns[0]).data("kendoDropDownList").value();
var dd2Value = $(dropdowns[1]).data("kendoDropDownList").value();
var dd3Value = $(dropdowns[2]).data("kendoDropDownList").value();

var answer1 = $('#idAnswer1').val();
var answer2 = $('#idAnswer3').val();
var answer3 = $('#idAnswer4').val();

and then later

{AnswerOne:answer1,AnswerTwo:answer2,AnswerThree,answer3,SecurityQuestionOne:dd1Value,SecurityQuestionTwo:dd2Value,SecurityQuestionThree:dd3Value}

you could instead have

var data={}, dropdowns = $("input.customdropdownlist");

for(var i=1; i<=6; i++){
     data['answer'+i]=$('#idAnswer'+i).val();
     data['question'+i]=$(dropdowns[i]).data("kendoDropDownList").value()
}

or

var data={};

$("input.customdropdownlist").each(function(dd,i){

     data['answer'+i]=$('#idAnswer'+i).val();
     data['question'+i]=$(dd).data("kendoDropDownList").value()
});

or one of the other many ways to iterate in JavaScript.

The same goes for your server-side code. Iterating through the data it receives to check it for blank fields will replace that long list of if statements with a short simple loop that will be much easier to read and maintain.

for(int i=1; i<=6; i++){
    string a=Request.QueryString["answer"+i];
    string q=Request.QueryString["question2+i];
}

Some quick remarks:

• AnswerOne, AnswerTwo: parameters should be camelCase

• AnswerOne != "" && AnswerOne != null: what’s wrong with String.IsNullOrEmpty?

• Int32.Parse(SecurityQuestionOne);: if you need ints, why not define the parameters as such?

• What is SecurityQuestions? I’m asking because class names shouldn’t be a plural.

• RequestSecurityQuestions_Submit: DO NOT use underscores, hyphens, or any other nonalphanumeric characters.

WRT the problem in general: I haven’t done any ASP.NET MVC, but surely you must be able to send a custom class/model of some sort instead of a dozen parameters? If you look at that link, you’ll see this code about halfway down the page:

[HttpPost]
public ActionResult Edit(Album album)

See also this SO question.