5
\$\begingroup\$

I'm using Cordova with jQuery and jQuery Mobile to develop a hybrid app. In this app I need to save data that I get from a server. The data is a JSON. I chose this SQLite plugin for Cordova to save the content in tables for easy access.

I make an AJAX-GET call to get the data I need and call the function below with the parameters: content is the JSON; myAction is a string(sync or update); upt is an integer(unix timestamp).

function syncQueries(content, myAction, upt){
    //JSON
    var uid = content.content[0].uid;
    var name = content.content[0].name;
    var now = Math.round(Date.now()/1000);
    if (typeof content.content[0].kurzbeschreibung != 'undefined'){
        var kurzbeschreibung = content.content[0].kurzbeschreibung;
    }
    if (typeof content.content[0].hauptkategorie != 'undefined'){
        var parentID = content.content[0].hauptkategorie;
    }
    if (typeof content.content[0].unterkategorie != 'undefined'){
        var parentID = content.content[0].unterkategorie;
    }
    if(content.content[0].beschreibung.match(/<img/)){
        var beschreibung = findImgTag(content.content[0].beschreibung);
    } else {
        var beschreibung = content.content[0].beschreibung;
    }
    //sync data
    if(myAction == 'update'){
        db.transaction(function(tx){
            if(content.content[0].tabelle  == 'hauptkategorie'){
                tx.executeSql("UPDATE hauptkategorie SET name = '"+name+"', beschreibung = '"+beschreibung+"', last_update = '"+now+"' WHERE pid = "+uid, [], function(tx){
                    tx.executeSql('UPDATE last_update_haupt SET update_time = '+upt+', last_success = '+now+', sync = 0 WHERE pid = '+uid ,[], function(){
                        sync++;
                    });
                    console.log(content.content[0].tabelle +': '+uid);
                });
            } else if(content.content[0].tabelle  == 'unterkategorie'){
                tx.executeSql("UPDATE unterkategorie SET name = '"+name+"', beschreibung = '"+beschreibung+"', parentid = '"+parentID+"', last_update = '"+now+"' WHERE pid = "+uid, [], function(tx){
                    tx.executeSql('UPDATE last_update_unter SET update_time = '+upt+', last_success = '+now+', sync = 0 WHERE pid = '+uid ,[], function(){
                        sync++;
                    });
                    console.log(content.content[0].tabelle +': '+uid);
                });
            } else if(content.content[0].tabelle  == 'loesung'){
                tx.executeSql("UPDATE loesung SET name = '"+name+"', kurzbeschreibung = '"+kurzbeschreibung+"', beschreibung = '"+beschreibung+"', subparentid = '"+parentID+"', last_update = '"+now+"' WHERE pid = "+uid, [], function(tx){
                    tx.executeSql('UPDATE last_update_loesung SET update_time = '+upt+', last_success = '+now+', sync = 0 WHERE pid = '+uid ,[], function(){
                        sync++;
                    });
                    console.log(content.content[0].tabelle +': '+uid);
                });
            }
        },function(e){
            console.error('ERROR: '+e.message);
            $.mobile.loading('hide');
        });
    } else if(myAction == 'sync'){
        db.transaction(function(tx){
            if(content.content[0].tabelle == 'hauptkategorie'){
                tx.executeSql('INSERT INTO hauptkategorie (pid, name, beschreibung, last_update) VALUES (?,?,?,?)', [uid, name, beschreibung, now], function(tx){
                    tx.executeSql('INSERT INTO last_update_haupt (pid, update_time, last_success, sync) VALUES (?,?,?,?)', [uid, upt, now, 0], function(){
                        sync++;
                    });
                    console.log(content.content[0].tabelle +': '+uid);
                });
            } else if(content.content[0].tabelle  == 'unterkategorie'){
                tx.executeSql('INSERT INTO unterkategorie (pid, name, beschreibung, parentid, last_update) VALUES (?,?,?,?,?)', [uid, name, beschreibung, parentID, now], function(tx){
                    tx.executeSql('INSERT INTO last_update_unter (pid, update_time, last_success, sync) VALUES (?,?,?,?)', [uid, upt, now, 0], function(){
                        sync++;
                    });
                    console.log(content.content[0].tabelle +': '+uid);
                });
            } else if(content.content[0].tabelle  == 'loesung'){
                tx.executeSql('INSERT INTO loesung (pid, name, kurzbeschreibung, beschreibung, subparentid, last_update) VALUES (?,?,?,?,?,?)', [uid, name, kurzbeschreibung, beschreibung, parentID, now], function(tx){
                    tx.executeSql('INSERT INTO last_update_loesung (pid, update_time, last_success, sync) VALUES (?,?,?,?)', [uid, upt, now, 0], function(){
                        sync++;
                    });
                    console.log(content.content[0].tabelle +': '+uid);
                });
            }
        },function(e){
            console.error('ERROR: '+e.message);
            $.mobile.loading('hide');
        });
    }
    //sync pictures
    if(content.content[0].grafik == 1){
        db.executeSql('UPDATE '+content.content[0].tabelle+' SET bild = 1 WHERE pid = '+content.content[0].uid,[],function(){
            file_info(content.content[0].tabelle, content.content[0].uid);
            console.log(content.content[0].uid+', '+content.content[0].tabelle);
        },function(e){
            console.error('ERROR: '+e.message);
            $.mobile.loading('hide');
        });
    } else {
        db.executeSql('SELECT * FROM '+content.content[0].tabelle+' WHERE pid = '+content.content[0].uid,[],function(res){
            if(res.rows.item(0).bild == 1){
                file_info(content.content[0].tabelle, content.content[0].uid);
            }
        },function(e){
            console.error('ERROR: '+e.message);
            $.mobile.loading('hide');
        },function(){
            syncEnd();
        });
    }
}

The function findImgTag takes a string, looks for <img> tags, downloads the images to the device storage and replaces them with a local path for offline usage. The return value is the string with replaces <img> tags.

Sync++ is to keep track of the amount of entries for the function syncEnd. syncEnd just hides the mobileLoader when the database is synchronized with the server.

The function fileInfo downloads pictures that are not inside the HTML text.


To ensure that the data is successfully inserted into the table I make an other INSERT into a different table that lists when an entry was updated(server side timestamp) and when the last successful insert was (now).

I read on Stack Overflow that you shouldn't make nested transactions but I haven't found an other way to ensure the INSERT or UPDATE was successful since jQuery is asynchronous.

My question: Is there a better way to do the INSERT and UPDATE and are there any other improvements I could make to the code?

\$\endgroup\$
3
  • \$\begingroup\$ Hi, I was going to do a more in depth review but I don't really know enough about your code so I think I'll just ask some thought provoking questions instead. 1) why SQL on the client side? why not just a map (object) and arrays? 2) what is the structure of content? \$\endgroup\$
    – Dan
    Commented Jul 17, 2015 at 19:29
  • \$\begingroup\$ 1)I wouldn't know how to save the object so I can reuse the data when the app is closed and reopened. 2){"content":[{"uid": integer, "name": string, "beschreibung": string, "grafik": integer, "hauptkategorie": integer, "deleted": integer, "timestamp": integer, "tabelle": string}]} \$\endgroup\$
    – user77908
    Commented Jul 17, 2015 at 19:41
  • \$\begingroup\$ Ah, right, cordova.. this is a different beast to what I am used to. \$\endgroup\$
    – Dan
    Commented Jul 17, 2015 at 19:43

1 Answer 1

1
\$\begingroup\$

Checking if variables are undefined, in my opinion, is better done like this:

if (content.content[0].kurzbeschreibung === void 0)

I can see why you wouldn't want to do that. Perhaps change the void 0 to undefined. But make sure to use triple equals unless you're sure you want the behavior of double equals. (see here).

String checking also should be done with triple equals (myAction === 'update' or myAction === 'sync').

I recommend using switch statements in the callbacks, instead of multiple if/else blocks. They'll make your code easier to both understand and change if you need to in the future.

Are you sure that the SQL queries you're building are safe? It looks like in some places you might be using user-provided data to build the queries, which is bad unless you escape them - not a trivial problem (to say the least). Although it looks like most of the data comes from your server or is whitelisted, make sure to be careful - I've accidentally SQL-injected myself before by returning bad data from a server I was working on.

Finally, the big one - I'd like to suggest that instead of using callbacks, you either wrap this library with Promises or switch to one that does that already. If you've never heard of promises, check this out. Right now, promises are only supported in a few browsers, but there are many libraries like Q and Bluebird that provide a lot of the same functionality, and there's an es6 shim for them as well. Promises can turn your code into somthing like this (including some of the other changes I suggested):

if (myAction === 'update'){
    db.transaction().then(function(tx) {
        switch (content.content[0].tabelle) {
        case 'hauptkategorie':
            return tx.executeSql("UPDATE hauptkategorie SET name = '" + name + "', beshreibung = '" + beschreibung + "', last_update = '" + now + "' WHERE pid = " + uid, []);
        case 'unterkategorie':
            return tx.executeSql("UPDATE unterkategorie SET name = '" + name + "', beschreibung = '" + beschreibung + "', parentid = '" + parentID + "', last_update = '" + now + "' WHERE pid = " + uid, []);
        case 'loesung':
            return tx.executeSql("UPDATE loesung SET name = '" + name + "', kurzbeschreibung = '" + kurzbeschreibung + "', beschreibung = '" + beschreibung + "', subparentid = '" + parentID + "', last_update = '" + now + "' WHERE pid = " + uid, []);
        }
    }).then(function(tx) {
        console.log(content.content[0].tabelle + ': ' + uid);
        switch (content.content[0].tabelle) {
        case 'hauptkategorie':
            return tx.executeSql('UPDATE last_update_haupt SET update_time = ' + upt + ', last_success = ' + now + ', sync = 0 WHERE pid = ' + uid, []);
        case 'unterkategorie':
            return tx.executeSql('UPDATE last_update_unter SET update_time = ' + upt + ', last_success = ' + now + ', sync = 0 WHERE pid = ' + uid, []);
        case 'loesung':
            return tx.executeSql('UPDATE last_update_loesung SET update_time = '+upt+', last_success = ' + now + ', sync = 0 WHERE pid = ' + uid, []);
        }
    }).then(function(tx) {
        sync++;
    }, function(error) {
        console.error('ERROR: ' + e.message);
        $.mobile.loading('hide');
    });
} else if (myAction === 'sync'){
    // similar here
}

If you are unfamiliar with the promise pattern, basically, a promise does something asynchronous, then calls the callback method provided by the then function. Since the then function returns a promise as well, you can chain together thens and make messy asynchronous code look like clean, synchronous code while retaining all of the benefits of asynchrony.

I only recommend using a Promise library since you are already using jQuery, and therefore I assume you can easily add more libraries on top of it. In fact jQuery provides its own (slightly worse) version of promises that it calls Deferreds.

\$\endgroup\$

Your Answer

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