3
\$\begingroup\$

I have a PhoneGap application that I wrote some time ago. After looking Doug Crockford's video seminar JavaScript: The Good Parts.

I was just wondering if the code could be improved for better maintainability and readability as it now could be hard for others to understand what's happening. Maybe taking advantage of using the module pattern and closures?

I know there's a big chunk of code in this post, so any help on implementing these patterns is well appreciated.

I'm pretty much following Rohit Ghatol's book Beginning PhoneGap code examples and design. That is, my app has the following execution order:

document.addEventListener('deviceready', onDeviceReady, false);

function onDeviceReady() {
    $(document).ready(function() {
        // Initiate the application
        bind();
    });
}

function bind() {
    initiateDatabases();
    initiateDashboardPage();
    initiateReserveBtn();
    initiateReservePage();
    initiateCentersBtn();
    initiateCentersPage();
    initiateDonationsBtn();
    initiateDonationsPage();
    initiateSettingsBtn();
    initiateSettingsPage();
    initiateQuestionsBtn();
    initiateQuestionsPage();
    initiateInformationBtn();
    initiateInformationPage();
    initiateMapPage();
}

In each of those initiateX() function I bind different jQuery Mobile page and button handling events, which will then call other functions. For example, initiateReservePage() looks like this:

function initiateReservePage() {    
    // When reserve page is shown
    $('#reserve-page').live('pageshow', function () {
        populateReserveList();           
    });

    var did_user_swipe  = false;

    // Bind mousedown/mouseup/mousemove events to 'refresh reserve' button
    $('#refreshReserveBtn').bind('vmousedown', function () {
        did_user_swipe  = false;
    }).bind('vmousemove', function() {
        did_user_swipe  = true;
    }).bind('vmouseup', function (e) {
        if(!did_user_swipe && e.which === 0) {  
            // DOWNLOAD THE RESERVE 
            downloadReserve();
        }
    });

    did_user_swipe      = null;
}

As it could be seen, that function either call downloadReserve() or populateReserve() function depending on the event being fired.

Could some sort of pattern be used here to combine those 2 functions?

Here are those 2 functions in question:

function downloadReserve() {    
    // URL to download reserve from
    var reserveURL          = 'http://myremotesite.com/file.xml';

    try {           
        $.ajax({
               type: 'GET',
               url: reserveURL,
               dataType: "xml",
               contentType: "text/xml; charset=utf-8",
               beforeSend: function (XMLHttpRequest) {
                   $.mobile.loadingMessage = 'Loading...';
                   $.mobile.showPageLoadingMsg();
               },
               complete: function(jqXHR) {
                    $.mobile.hidePageLoadingMsg();
               },
               success: function(data, textStatus, jqXHR) {
                    // This is a global variable to cache the fetched data
                    cachedReserve   = [];

                    // Reserve descriptions for different states
                    var reserveDescription  =   ['Critical', 'Adequate', 'Normal', 'Good'],
                        // Reserve item from XML
                        $item,
                        // Group for item
                        group = '',
                        // State for item
                        state = '',
                        // Description for item
                        desc = '',
                        // Reserve object for item
                        tmpReserveObj = {};

                    $(data).find('item').each(function() {
                        $item       = $(this);
                        group       = $item.find('group').text();
                        state       = $item.find('state').text();

                        if (state   == 'A') {
                            desc    = reserveDescription[0];
                        } else if (state == 'B') {
                            desc    = reserveDescription[1];
                        } else if (state == 'C') {
                            desc    = reserveDescription[2];
                        } else if (state == 'D') {
                            desc    = reserveDescription[3];
                        }

                        // Create a temporary reserve object
                        tmpReserveObj = { 'group':group, 'state':state, 'desc':desc };

                        // Push temporary object into global cached array
                        cachedReserve.push(tmpReserveObj);
                    });

                    $item                        = null;
                    group                       = null;
                    state                       = null;
                    desc                        = null;
                    tmpReserveObj           = null;
                    reserveDescription      = null;

                    // Insert reserve array into local storage and populate list
                    insertReserve(cachedReserve);

               },
               error: function (xhr, ajaxOptions, thrownError){    
                    // Show an alert here
               }
        });     
    } catch (e) {
        console.log('Error while performing ajax get call to download the reserve ' + e);
    }

    reserveURL              = null;
}

...and...

function populateReserveList() {

    try {
        // Get the reserve from local storage
        var reserve = JSON.parse(window.localStorage.getItem('reserve'));
    } catch (e) {
        alert('error getting the reserve from local storage: ' + e);
        return false;
    }

    if (reserve !== null && reserve.length > 0) {

        // If more than a week old
        if (isReserveDataOld()) {

            $.mobile.hidePageLoadingMsg();

            // Show alert box to ask user whether to download reserve again
            navigator.notification.confirm(
                'Your downloaded reserve seems to be old. Do you want to download it again?',
                function(buttonIndex) {
                    // If 'OK' button clicked
                    if (buttonIndex === 2) {
                        // DOWNLOAD THE RESERVE AGAIN
                        downloadReserve();
                    }
                },
                'Download the reserve',
                'Cancel,OK');

        } else {
            // Reserve list container
            var list                = document.getElementById('reserveList'),
                // Timestamp for when reserve was inserted into local storage
                timestamp           = '',
                // Date based on timestamp
                _date;

            // Empty the reserve list container and remove all event handlers from child elements
            $(list).empty();

            try {
                timestamp           = window.localStorage.getItem('reserveInserted');
                timestamp           = ((timestamp != null && timestamp != 'undefined' && timestamp.length > 0) ? timestamp : 0);
                _date               = new Date(timestamp - 0);
            } catch (e) {
                alert('Error while getting reserveInserted from local storage ' + e);
            }

            var infoBarText         =   '<h3>Downloaded' +
                                        '<span>' +_date.getDate() + '. ' + monthNames[_date.getMonth()] + 'ta ' + _date.getFullYear()+ '</span></h3>';

            var infoBarElem         = document.createElement('div');
            infoBarElem.setAttribute('id', 'infoBar');
            infoBarElem.className   = 'downloaded';
            infoBarElem.innerHTML   = infoBarText;
            list.appendChild(infoBarElem);

            infoBar                 = null;
            infoBarText             = null;

            var reserveDetailsContainer         = document.createElement('div');
            var reserveListContainer            = document.createElement('div');
            var reserveHeadingElem              = document.createElement('h3');

            reserveDetailsContainer.className   = 'detailsContainer';
            reserveListContainer.className      = 'listContainer';

            list.appendChild(reserveDetailsContainer);
            list.appendChild(reserveListContainer);

            // Global variable to cache reserve data
            cachedReserveData   = [];

            var htmlData            = '';
            var reserveHeadingContainer;

            var did_user_swipe      = false;

            // Iterate through all reserve items
            for ( var index = 0 ; index < reserve.length ; index++ ) {
                var item    = reserve[index];

                cachedReserveData.push(item);

                reserveHeadingContainer                 = reserveHeadingElem.cloneNode(false);
                reserveHeadingContainer.setAttribute('id', index);
                reserveHeadingContainer.setAttribute('name', item.group.toLowerCase());

                htmlData                                = 'Group: ' + item.group + '<span>Show group</span>';
                reserveHeadingContainer.innerHTML       = htmlData;
                reserveHeadingContainer.className       += ' state_' + item.state;
                reserveListContainer.appendChild(reserveHeadingContainer);

                $(reserveHeadingContainer).bind('vmousedown', function(event) {
                    did_user_swipe = false;                                     
                }).bind('vmousemove', function () {
                    did_user_swipe = true;
                }).bind('vmouseup', function(e) {

                    // If triggered manually or by user
                    if (passDataBetweenPages == 'trigger' || (!did_user_swipe && e.which == 0)) {

                        try {                           
                            var id              = parseInt($(this).attr('id'));

                            // Empty the reserve details container and remove all event handlers from child elements
                            $(reserveDetailsContainer).empty();                         

                            htmlData        =   '<div>' +
                                                '<p>' +cachedReserveData[id].desc+ '</p>' +
                                                '</div>';

                            // Replace contentContainer's content with clicked list items's data
                            reserveDetailsContainer.innerHTML       = htmlData;

                            var newHeading  = $(this).clone();
                            newHeading.find('span').html('State');
                            $(reserveDetailsContainer).prepend(newHeading);

                            $(reserveListContainer).find('h3.active').removeClass('active');
                            $(this).addClass('active');

                            newHeading  = null;

                            // Scroll to top
                            scroll(0, 0);

                        } catch (e) {
                            alert('Error while clicking listItem with id = ' + $(this).attr('id') + ' ' + e);
                        }
                    }
                });
            }

            // Get group from local storage
            var group           = getItemFromLocalStorage('group');

            // Open corresponding group or if not found, the first item on the list
            if (group && group != '0') {
                var listItem        = $(reserveListContainer).find('h3[name="' +group+ '"]').get(0);
                passDataBetweenPages    = 'trigger';
                $(listItem).trigger('vmouseup');
                passDataBetweenPages    = null;
                listItem                = null;
            } else {
                var listItem            = $(reserveListContainer).find('h3').get(0);
                passDataBetweenPages    = 'trigger';
                $(listItem).trigger('vmouseup');
                passDataBetweenPages    = null;
                listItem                = null;
            }

            group               = null;
        }
    } else {
        $.mobile.hidePageLoadingMsg();

        // Show alert box to ask user whether to download the reserve
        navigator.notification.confirm(
            'The reserve has not been downloaded yet. Do you want to download it now?',
            function(buttonIndex) {
                console.log(buttonIndex);
                if (buttonIndex === 2) {
                    // DOWNLOAD THE RESERVE
                    downloadReserve();
                }
            },
            'Download the reserve',
            'Cancel,OK');
    }
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

I think your code looks fine (except for populateReserveList).

  • I would look into replacing .live() calls with .on() calls as .live() has been deprecated since a while.
  • Your success function is so large, it ought to be a function on it's own
  • You could put the state -> description mapping in an object

    var stateDescriptionMap = { 'A' : 0 , 'B' : 1 , 'C' : 2 , 'D' : 3 }
    

    and then

    desc = reserveDescription[ stateDescriptionMap[ state ] ];
    
  • I am not sure what you want to accomplish with ( your code should work without this? )

                $item                        = null;
                group                       = null;
                state                       = null;
                desc                        = null;
                tmpReserveObj           = null;
                reserveDescription      = null;
    
  • populateReserveList is far too large and has to be broken up in logical parts.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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