Creating object in JS that is passed into IHttpHandler

Question

I am working on a web page where I am creating a JS object using {} and []. This object is passed into a handler

I wanted to see if there is any more way to write this method. Its a lot of code and just wanted to make sure there is no better way to do this?

My logic

Is to build the object is JS and then pass it into the Handler to be Deserialize and then saved into the db.

function saveLongAnswerText() {
 console.log('%c saveLongAnswerText ', 'background: #222; color: #bada55');
 if ($("#form1").valid()) {
 MeetingPollingQuestion = {};
 MeetingPollingQuestion.MeetingPollingId = $("#hfMeetingPollingId").val();
 MeetingPollingQuestion.MeetingPollingQuestionType = "LongAnswerText";
 MeetingPollingQuestion.SequenceOrder = 2;
 MeetingPollingQuestion.MeetingPollingParts = [];
 MeetingPollingParts = {};
 MeetingPollingParts.Type = "Question";
 MeetingPollingParts.MeetingPollingPartsValues = [];
 MeetingPollingPartsValues = {};
 MeetingPollingPartsValues.Type = "Question";
 MeetingPollingPartsValues.QuestionValue = $("#editorLongAnswerText").data("kendoEditor").value();
 MeetingPollingParts.MeetingPollingPartsValues.push(MeetingPollingPartsValues);
 MeetingPollingQuestion.MeetingPollingParts.push(MeetingPollingParts);
 console.log(MeetingPollingQuestion);
 Metronic.blockUI({ boxed: true, message: "Saving Question.." });
 $.ajax({
 type: 'POST',
 url: 'ManagePolling.ashx',
 data: { "PollingQuestion": JSON.stringify(MeetingPollingQuestion), "Action": "SaveQuestion" },
 datatype: "JSON",
 success: function (data) {
 if (data.resultStatus.ResultCode == "1") {
 toastr.success("Saved successfully", "Success");
 }
 if (data.resultStatus.ResultCode == "2")
 toastr.warning(data.resultStatus.Message, "Warning");
 if (data.resultStatus.ResultCode == "3")
 toastr.warning(data.resultStatus.Message, "Error");
 Metronic.unblockUI();
 },
 error: function (data) {
 toastr.error('An error has occured! \n' + data.resultStatus.Message);
 Metronic.unblockUI();
 }
 });
 }
}

Handler

 string jsonData = context.Request.Form["PollingQuestion"];
 var MeetingPollingQuestion = new Model.MeetingPollingQuestion();
 var MeetingPollingParts = new List<Model.MeetingPollingParts>();
 var MeetingPollingPartsValues = new List<Model.MeetingPollingPartsValues>();
 MeetingPollingQuestion.MeetingPollingParts = MeetingPollingParts;
 MeetingPollingQuestion = Deserialize<Model.MeetingPollingQuestion>(jsonData);

• BCdotWEB: Is this correct: MeetingPollingPartsValues.Type = "Question";?
• Jefferson: I think that will be changed at some point.
• Peter Csala: From what perspective are you expecting a review?
• Jefferson: How I could possibly improve the code quality?
• Peter Csala: @Jefferson Thank you for the bounty! :)

Answer 1

Client-side

Prefer fail fast

• Rather than guarding the whole object creation and the ajax communication logic with a condition if ($("#form1").valid()) please prefer early exit
  • if (!$("#form1").valid()) return;

Prefer object and collection initializer

• Rather than using value assignment you can use object initializer to avoid repetitive code

const MeetingPollingQuestion = {
 MeetingPollingId: $("#hfMeetingPollingId").val(),
 MeetingPollingQuestionType: "LongAnswerText",
 SequenceOrder: 2,
 MeetingPollingParts: [
 {
 Type: "Question",
 MeetingPollingPartsValues: [
 {
 Type: "Question",
 QuestionValue: $("#editorLongAnswerText").data("kendoEditor").value()
 }
 ]
 }
 ]
};

Try to avoid mixing stringify and object initializer

• This line seems really odd to me

data: { "PollingQuestion": JSON.stringify(MeetingPollingQuestion), "Action": "SaveQuestion" }

• I would suggest to use object initializer first and then stringify it

data: JSON.stringify({ PollingQuestion: MeetingPollingQuestion, Action: "SaveQuestion" })

Naming

• I would suggest to consolidate your naming strategy because even in a simple line you have Pascal and camel Casing as well

data.resultStatus.ResultCode

• It is minor but please also try to consolidate the usage of ' and "

Please prefer switch

• Rather than having 3 if branches (.ResultCode == "1" ... .ResultCode == "3") please prefer to use switch

switch (data.resultStatus.ResultCode) {
 case "1": toastr.success("Saved successfully", "Success"); break;
 case "2": toastr.warning(data.resultStatus.Message, "Warning"); break;
 case "3": toastr.warning(data.resultStatus.Message, "Error"); break;
}

Server-side

• The MeetingPollingPartsValues variable is not used
• I think the MeetingPollingParts variable is not needed because you overwrite the entire MeetingPollingQuestion object
• So, the entire method could be simplified for the following line

var meetingPollingQuestion = Deserialize<Model.MeetingPollingQuestion>(context.Request.Form["PollingQuestion"]);

• Jefferson: can I do a loop in the object initializer?
• Peter Csala: @Jefferson As far as I know, you can not
• Jefferson: I added that Addition object would I not be able to create the const ?
• Peter Csala: @Jefferson Here at code review we don't modify / extend the original question after a review has been placed. We encourage people to create a follow up question and link them.
• Jefferson: I created a new post with that method codereview.stackexchange.com/questions/280364/…