HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Improving PhoneGap/JavaScript application

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
javascriptphonegapimprovingapplication

Problem

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) {

Solution

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.

Code Snippets

var stateDescriptionMap = { 'A' : 0 , 'B' : 1 , 'C' : 2 , 'D' : 3 }
desc = reserveDescription[ stateDescriptionMap[ state ] ];
$item                        = null;
            group                       = null;
            state                       = null;
            desc                        = null;
            tmpReserveObj           = null;
            reserveDescription      = null;

Context

StackExchange Code Review Q#13862, answer score: 2

Revisions (0)

No revisions yet.