Ajax call to update fields on the HTML page, populate the dropdown-menu, table, and piechart

Question

The purpose of this file is to make an Ajax call to a certain endpoint, and update a certain fields on the HTML page, auto populate the dropdown-menu, table, and piechart.

My script will turn this:

enter image description here

to this:

enter image description here

I'm not sure whether or not I should do all my logic within my Ajax success. Should I separate it? This is what I have. Is it really that bad?

'use strict';
define(['jquery', 'moment'], function(,ドル moment) {
 $(function() {
 var report_type_car = $('#as-report-type-car');
 var report_type_cdr = $('#as-report-type-cdr');
 var report_type_title = $('#as-report-type-title');
 var section_num = $('#as-section-num');
 var problem_set = $('#as-problem-set');
 var start_time = $('#as-start-time');
 var due_time = $('#as-due-time');
 var student_am = $('#as-student-am');
 var student_total = $('#as-student-total');
 var submit_am = $('#as-submit-am');
 var submit_total = $('#as-submit-total');
 var avg_score = $('#as-avgscore');
 var danger = $('#pc-danger');
 var warning = $('#pc-warning');
 var success = $('#pc-success');
 var danger_list = $('#pc-danger-list');
 var warning_list = $('#pc-warning-list');
 var success_list = $('#pc-success-list');
 //var basePath = "/BIM/resources/js/reports/teacher/section-exercise/assignment/";
 $.ajax({
 url: "/BIM/rest/report/assignment",
 type: "POST",
 dataType: "json",
 data: {
 assessmentId: "206a9246-ce83-412b-b8ad-6b3e28be44e3",
 classroomId: "722bfadb-9774-4d59-9a47-89ac9a7a8f9a"
 },
 success: function(objects) {
 var x = 0;
 function updateInfo(x) {
 var json = objects.assignments[x].header;
 var name = objects.assignments[x].name;
 // Check for space in report_type
 if (json.report_type.indexOf(' ') >= 0) {
 var report_type_full = json.report_type.split(/(\s+)/);
 var car = report_type_full[0];
 var cdr = report_type_full[2];
 report_type_car.html(car);
 report_type_cdr.html(cdr);
 report_type_title.html(car + " " + cdr + " " + json.section_num);
 } else {
 var car = json.report_type;
 report_type_car.html(car);
 report_type_title.html(car + " " + json.section_num);
 }
 section_num.html(json.section_num);
 problem_set.html(json.problem_set);
 // Not show date if summary
 if (name == "Summary") {
 start_time.html(" ");
 due_time.html(" ");
 $("#as-due-time-div").hide();
 $("#as-start-time-div").hide();
 } else {
 $("#as-due-time-div").show();
 $("#as-start-time-div").show();
 start_time.html(moment(parseInt(json.start_time)).format("MM/DD/YYYY h:mm A"));
 due_time.html(moment(parseInt(json.due_time)).format("MM/DD/YYYY h:mm A"));
 }
 student_am.html(json.student_am);
 student_total.html(json.student_total);
 submit_am.html(json.submit_am);
 submit_total.html(json.submit_total);
 avg_score.html(json.avg_score);
 danger.html(json.danger);
 warning.html(json.warning);
 success.html(json.success);
 danger_list.html(json.danger_list);
 warning_list.html(json.warning_list);
 success_list.html(json.success_list);
 }
 // Load the Visualization API and the piechart package.
 google.load("visualization", "1", {
 packages: ["corechart"]
 });
 google.setOnLoadCallback(drawChart());
 function drawChart() {
 var options = {
 width: 160,
 height: 160,
 chartArea: {
 left: 10,
 top: 20,
 width: "100%",
 height: "100%"
 },
 colors: ['#F46E4E', '#F9C262', '#ADB55E', ],
 legend: 'none',
 enableInteractivity: false,
 pieSliceText: 'none',
 };
 var chart = new google.visualization.PieChart(document.getElementById('piechart'));
 var data = {};
 $.each(objects.assignments, function(i, v) {
 console.log(v);
 var header = v.header;
 var name = v.name;
 var total = header.danger + header.warning + header.success;
 data[i] = new google.visualization.arrayToDataTable([
 ['Piechart', 'Number of Skills'],
 ['danger', (header.danger / total) * 100],
 ['warning', (header.warning / total) * 100],
 ['success', (header.success / total) * 100],
 ]);
 // Auto Populate the dropdown-menu
 if (i == 0) {
 $("#as-dd.dropdown").append('<option value="' + name + '"> Summary </option>');
 } else {
 $("#as-dd.dropdown").append('<option value="' + name + '">' + name + '</option>');
 }
 });
 // Dropdown-menu change
 $('#as-dd').on('change', function() {
 var value = $(this).val();
 updateInfo(value);
 chart.draw(data[value], options);
 });
 // Initializing
 updateInfo("0");
 chart.draw(data[0], options);
 }
 }
 });
});
});

• cFreed: When you ask if you should do all logic within Ajax success, it may mean two different things. First, physically speaking: IMO you might consider to locate your updateInfo() and drawChart() functions outside of the Ajax success:, for readalility. This way, the main body of the success process appears more clearly. Then, logically speaking: I didn' take time to precisely understand how this works, but I guess you should have some code (may be the Initializing sequence?) outside of the success: part, something like a always:part, to cover the case of potential errors.

Answer 1

I'm agree with the improvements suggested by @cfreed, you should extract updateInfo() and drawChart() outside of the ajax call. This should be a better split btw the data/view. A side effect, could be doesn't have all jquery selectors on the top of the request.

Also, I would like add my owns:

• If you are using $.ajax with that configuration, you can cosidere use the shortwhand mehtods like $.post
• is a good convention use camelCase instead of sanake_case for variable names.

• you should consider use some kind of default value for the assigments:

  var json = objects.assignments[x].header || {} not always the server response with the full data. Asume null values and considere to use null object pattern.

• Be more expresive in name variables:

  $.each(objects.assignments, function(i, v) {//who is v?