Loading details of a list of cars

Question

I have a tree view with a list of cars and a webservice to get the details for each car.

On page load, there are no cars selected so the details footer is loaded with all cars.

$.ajax({
        type: 'POST',
        url: '@Url.Content("/GTracker/Cars/CarTreeDetails")',
        data: { "id": [] }, // not cars filter
        traditional: true,
        success: function (data) {
            $('#footer-details').html(data);
        }
    });

When the user select cars from the treeview, the details are then updated with only those cars.

This is the changed.jstree event:

    var ajaxDetails;

    $('#carTree').on('changed.jstree', function (e, data) {
        var i, j, p = [];

        // create the id[] array 
        for (i = 0, j = data.selected.length; i < j; i++) {
            node = data.instance.get_node(data.selected[i]);
            if (node.data.type != 'root') {
                p.push(parseInt(data.instance.get_node(data.selected[i]).id));
            }
        }

        // cancel previous request
        if ((ajaxDetails && ajaxDetails.readyState != 4)) {
            ajaxDetails.abort()
        }

        // get only selected cars info.
        ajaxDetails = $.ajax({
            type: 'POST',
            url: '@Url.Content("/GTracker/Cars/CarTreeDetails")',
            data: { "id": p },  //cars list
            traditional: true,
            success: function (data) {
                $('#footer-details').html(data);
            }
        });
    });

But I feel I'm repeating myself. How may I merge those two functions together?

• one for the on_load.
• the other for the js_tree change event.

Answer 1

I think this is a good case for refactoring the ajax call out into its own method. I have given some suggested changes in the code below. My comments are inside /* ... */ comments.

/*
 * use more meaningful and specific name
var ajaxDetails;
*/
var carTreeXHR;

$('#carTree').on('changed.jstree', function (e, data) {
    /*
     * why initialize to i and j here?
     * use more meaningful name for selected ID's array
    var i, j, p = [];
    */
    var selectedIds = [];

    // create the id[] array
    /*
     * initialize i and j here.
     * simplify call to get node id from existing 'node' object
    for (i = 0, j = data.selected.length; i < j; i++) {
        node = data.instance.get_node(data.selected[i]);
        if (node.data.type != 'root') {
            p.push(parseInt(data.instance.get_node(data.selected[i]).id));
        }
    }
    */
    for (var i = 0, j = data.selected.length; i < j; i++) {
        var node = data.instance.get_node(data.selected[i]);
        if (node.data.type != 'root') {
            selectedIds.push(parseInt(node.id));
        }
    }
    /*
     * remove from this location and put inside  new function
    // cancel previous request
    if ((ajaxDetails && ajaxDetails.readyState != 4)) {
        ajaxDetails.abort()
    }
    */

    // get only selected cars info.
    /* call new function */
    updateCarTreeDetails(selectedIds);

   /*
   * remove to function
   ajaxDetails = $.ajax({
        type: 'POST',
        url: '@Url.Content("/GTracker/Cars/CarTreeDetails")',
        data: { "id": p },  //cars list
        traditional: true,
        success: function (data) {
            $('#footer-details').html(data);
        }
    });
    */
});

/* New function. Include in appropriate scope */
function updateCarTreeDetails(selectedIds) {
    // abort existing request if needed
    if ((carTreeXHR && carTreeXHR.readyState != 4)) {
        carTreeXHR.abort()
    }
    // execute new request
    // not shown - consider handling ajax failure case as well
    carTreeXHR = $.ajax({
        type: 'POST',
        url: '@Url.Content("/GTracker/Cars/CarTreeDetails")',
        data: { "id": selectedIds },  //cars list
        traditional: true,
        success: function (data) {
            $('#footer-details').html(data);
        }
    });
}

/* In your document.ready handler, you can call this function
 * just like you did inside the change handler */
updateCarTreeDetails([]);

One additional note - it seems odd to be executing a 'POST' method if a case where you are just reading data from the remote data source (i.e. you are not performing a create, update, or delete operation). If you have control over this API, you might consider implementing as GET.