0
\$\begingroup\$

I have written this implementation and I would love to make it more efficient.

While I am indeed requesting a code review/suggestion, I would also love to get feedback on readability and coding style. Every bit of input helps

The main code for this factory is below:

(function(window, q, ,ドル cordova) {
 "use strict";
 //Database Object and properties
 var _db = {
 instance: null,
 transactionTypes: {
 readonly: "readonly",
 readwrite: "readwrite"
 },
 /// <summary>
 /// Error Handling/Logging/Redirection handled here
 /// </summary>
 utilityMethods: {
 logErrors: function(error) {
 //Log errors
 //ToDo: Implement local and api strategies for error handling
 }
 },
 /// <summary>
 /// Check if SQLite is available
 /// </summary>
 ///<observation>
 /// This is only applicable for scenarios where 
 /// user has denied sqlite quota/application quota
 ///</observation>
 initDatabase: function() {
 var deferred = q.defer();
 try {
 if (!window.openDatabase) // Check device supports SQLite or not.
 {
 deferred.reject(new Error("Databases are not supported in this device."));
 } else {
 setTimeout(function() { deferred.resolve("Database Created Successfully"); }, 100);
 }
 } catch (e) {
 if (e === 2) {
 // Version number mismatch. 
 deferred.reject(new Error("Invalid Version of Database"));
 } else {
 deferred.reject(new Error(e));
 }
 }
 return deferred.promise;
 },
 /// <summary>
 /// Open a Specific SqLite Database
 /// </summary>
 /// <parameters>
 /// Database Model Object (define in app)
 /// </parameters>
 ///<observation>
 /// Init Handler for Begining transactions
 ///</observation>
 open: function(databaseModel) {
 var deferred = q.defer();
 try {
 var instance = openDatabase(databaseModel.name, databaseModel.version, databaseModel.alias,
 databaseModel.timeout); //Open a DB based on Name and Version Number
 _db.instance = instance;
 deferred.resolve(instance);//This is the db instance we will use in every single query
 } catch (e) {
 deferred.reject(new Error(e));
 }
 return deferred.promise;
 },
 /// <summary>
 /// Check if Db Exists
 /// </summary>
 /// <parameters>
 /// The Table Name to Test for
 /// </parameters>
 ///<observation>
 /// Applicable for Android only
 ///</observation>
 checkDbExists: function(dbName) {
 var deferred = q.defer();
 // CHECK IF SQLite/Legacy DATABASE EXISTS. 
 //IF DOES EXPORT EXISTING DATA TO NEW DB THEN DELETE LEGACY
 var request = window.resolveLocalFileSystemURL(cordova.file.applicationStorageDirectory +
 "/databases/<DatabaseName>.db");
 try {
 _db.instance = request;
 deferred.resolve(true);
 } catch (e) {
 deferred.reject(new Error(e));
 }
 return deferred.promise;
 },
 /// <summary>
 /// Create Statements Factory 
 /// </summary>
 /// <parameters>
 /// The Table Name to Create
 /// </parameters>
 createTable: function(createStatement) {
 var deferred = q.defer();
 try {
 var request = _db.instance.transaction(function(tx) { tx.executeSql(createStatement, []); });
 deferred.resolve(request);//Resolves as an SQL Resultset, no cursor support
 } catch (e) {
 deferred.reject(new Error(e));
 }
 return deferred.promise;
 },
 /// <summary>
 /// Update Statements Factory 
 /// </summary>
 /// <parameters>
 /// The actual Update Statement, 
 /// the array of data elements to Update 
 /// </parameters>
 updateTable: function (updateStatement, dataArray) {
 var deferred = q.defer();
 try {
 var request = _db.instance.transaction(function (tx) {
 tx.executeSql(updateStatement, dataArray,
 function (tx, results) {
 deferred.resolve(results); //now it is ready to be queried against
 }, function(error) {
 deferred.reject(new Error(error));
 });
 });
 } catch (e) {
 deferred.reject(new Error(e));
 }
 return deferred.promise;
 },
 /// <summary>
 /// Insert Statements Factory 
 /// </summary>
 /// <parameters>
 /// The actual Insert Statement, 
 /// the array of data elements to Insert
 /// </parameters>
 insertTable: function(insertStatement, dataArray) {
 var deferred = q.defer();
 try {
 var request = _db.instance.transaction(function(tx) {
 tx.executeSql(insertStatement, dataArray,
 function(tx, results) {
 var lastInsertId = results.insertId; // this is the id of the insert just performed
 deferred.resolve(lastInsertId); //now it is ready to be queried against
 }, function (tx, error) {
 _db.utilityMethods.logErrors(error);
 deferred.reject(new Error(error));
 });
 });
 } catch (e) {
 deferred.reject(new Error(e));
 }
 return deferred.promise;
 },
 /// <summary>
 /// Insert Statements Factory 
 /// </summary>
 /// <parameters>
 /// The actual Insert Statement, 
 /// the array of data elements to Insert
 /// </parameters>
 insertTableNoPromise: function (insertStatement, dataArray) {
 try {
 var request = _db.instance.transaction(function (tx) {
 tx.executeSql(insertStatement, dataArray,
 function (tx, results) {
 var lastInsertId = results.insertId;
 return lastInsertId; // this is the id of the insert just performed
 }, function (tx, error) {
 _db.utilityMethods.logErrors(error);
 });
 });
 } catch (e) {
 }
 },
 /// <summary>
 /// Bulk Insert Statements Factory 
 /// </summary>
 /// <parameters>
 /// The actual Insert Statement, 
 /// the array of data elements to Insert
 /// This method accepts entire arrays of data for mass inserts
 /// </parameters>
 insertTableArray: function(insertStatement, dataArray) {
 //There are multiple steps in this code
 //Step 1 Get an array of Insert parameters 
 //From the Insert Statement
 function getFilterString(str) {
 var left = str.indexOf('(');
 var right = str.indexOf(')');
 var retVal = str.substring(left + 1, right);
 //Remove white spaces from the string
 return retVal.replace(/\s+/g, '');
 }
 var filterString = getFilterString(insertStatement);
 var filterArray = filterString.split(',');
 var deferred = q.defer();
 var request = _db.instance.transaction(function(tx) {
 var n = dataArray.length;
 //Step 2 Remove properties from the array that are not needed (updated date for example)
 //This is based on the Filter Array and needed for insert statement to work
 _db.instance.transaction(function(tx) {
 var transformedRecords = dataArray.map(dataArray => filterArray.reduce((p, c) => {
 p[c] = dataArray[c];
 return p;
 }, {}));
 //Saving the array in the db
 var saveData = function(dataArray, k) {
 //Step 3 Select and map individual records 
 var array = $.map(transformedRecords[k], function(value, index) {
 return [value];
 });
 try {
 //Step 4 Insert Data into the database
 tx.executeSql(insertStatement, array,
 function(tx) {
 if (k < (dataArray.length - 1)) {
 //All records arent saved yet, 
 //recall inner function to continue saving
 saveData(dataArray, (k + 1));
 } else {
 //We are done, send resolved code 
 //along with # of records saved
 //match saved number with initial sent number 
 //to check if all records were saved
 deferred.resolve(k);
 }
 },
 function(tx, error) {
 _db.utilityMethods.logErrors(error);
 deferred.reject(new Error(error.code));
 if (k < (dataArray.length - 1)) {
 saveData(dataArray, (k + 1));
 }
 });
 } catch (e) {
 //Legacy OS or browser advanced 
 //array functionality is not supported
 _db.utilityMethods.logErrors(e.code);
 }
 }
 //Call save data function with 
 //filtered records and set seed of 0
 saveData(transformedRecords, 0);
 });
 });
 return deferred.promise;
 },
 /// <summary>
 /// Select Statements Factory 
 /// </summary>
 /// <parameters>
 /// The actual Select Statement, 
 /// the array of data elements to select 
 /// The table name to verify it exists
 /// </parameters>
 selectTable: function(selectStatement, dataArray, tableName) {
 var deferred = q.defer();
 var existsQuery = "SELECT * FROM sqlite_master WHERE name = '" + tableName + "' and type='table'";
 try {
 var checkTable = _db.instance.transaction(function(tx) {
 tx.executeSql(existsQuery, [], function(tx, result) {
 if (result.rows.length === 0) {
 //No Such Table Exists
 deferred.reject();
 } else {
 //Table Exists try Selecting
 var request = _db.instance.transaction(function(tx) {
 if (dataArray.length) {
 tx.executeSql(selectStatement, dataArray, function(tx, result) {
 if (result.rows.length === 0) {
 //No Records Exists
 deferred.reject();
 } else {
 //Records Found
 deferred.resolve(result);
 }
 }, function (tx, error) {
 _db.utilityMethods.logErrors(error);
 deferred.reject(new Error(error));
 });
 } else {
 tx.executeSql(selectStatement, [], function(tx, result) {
 if (result.rows.length === 0) {
 //No Records Exists
 deferred.reject();
 } else {
 //Records Found
 deferred.resolve(result);
 }
 },function(error) {
 _db.utilityMethods.logErrors(error);
 deferred.reject(new Error(error));
 }
 );
 }
 });
 }
 });
 });
 } catch (e) {
 deferred.reject(new Error(e));
 }
 return deferred.promise;
 },
 /// <summary>
 /// Returns a count of records matching a foreign key
 /// </summary>
 /// <parameters>
 /// The actual count statement Statement, 
 /// the Object Callback, use this to pass in a parent table
 /// Passing in a parent object will return a count of child object 
 /// </parameters>
 selectCount: function(selectStatment, callbackData) {
 var deferred = q.defer();
 try {
 var request = _db.instance.transaction(function(tx) {
 tx.executeSql(selectStatment, [], function(tx, result) {
 if (result.rows.length === 0) {
 //No Records Exists
 deferred.reject();
 } else {
 //Records Found
 if (callbackData) { //Select Count Operations
 var objResponse =
 {
 count: result.rows[0].Count,
 callbackData: callbackData
 }
 deferred.resolve(objResponse);
 } else {
 deferred.resolve(result.rows[0].Count);
 }
 }
 }, function(error) {
 _db.utilityMethods.logErrors(error);
 deferred.reject(new Error(error));
 }
 );
 });
 } catch (e) {
 deferred.reject(new Error(e));
 }
 return deferred.promise;
 },
 /// <summary>
 /// Delete By ID Statements Factory 
 /// </summary>
 /// <parameters>
 /// The actual Delete Statement, 
 /// the ID of elements to delete
 /// The table name to verify it exists
 /// </parameters>
 deleteRowById: function(deleteStatement, dataArray, tableName) {
 var deferred = q.defer();
 var existsQuery = "SELECT * FROM sqlite_master WHERE name = '" + tableName + "' and type='table'";
 try {
 var checkTable = _db.instance.transaction(function(tx) {
 tx.executeSql(existsQuery, [], function(tx, result) {
 if (result.rows.length === 0) {
 //No Such Table Exists
 deferred.reject();
 } else {
 //Table Exists try Selecting
 var request = _db.instance.transaction(function(tx) {
 if (dataArray.length) {
 tx.executeSql(deleteStatement, dataArray, function(tx, result) {
 if (result.rows.length === 0) {
 //No Records Exists
 deferred.reject();
 } else {
 //Records Found
 deferred.resolve(result);
 }
 });
 }
 }, function (tx, error) {
 _db.utilityMethods.logErrors(error);
 deferred.reject(new Error(error));
 });
 }
 });
 });
 } catch (e) {
 deferred.reject(new Error(e));
 }
 return deferred.promise;
 },
 /// <summary>
 /// Delete All Rows Factory Method
 /// </summary>
 /// <parameters>
 /// The Table Name
 /// </parameters>
 deleteAllRows: function(tableName) {
 var deferred = q.defer();
 var deleteStatement = "DELETE from " + tableName;
 try {
 var request = _db.instance.transaction(function(tx) {
 tx.executeSql(deleteStatement, function() {
 alert("success");
 });
 }, function ignoredError(err) {
 deferred.resolve("Success"); //No Records were available to delete
 }, function() {
 deferred.resolve("Success"); //No Records were available to delete
 });
 } catch (e) {
 deferred.resolve("Success"); //No Records were available to delete
 }
 return deferred.promise;
 },
 /// <summary>
 /// Drop Table Factory method
 /// </summary>
 /// <parameters>
 /// The Table Name
 /// </parameters>
 dropTable: function(tableName) {
 var deferred = q.defer();
 var existsQuery = "SELECT * FROM sqlite_master WHERE name = '" + tableName + "' and type='table'";
 try {
 var checkTable = _db.instance.transaction(function(tx) {
 tx.executeSql(existsQuery, [], function(tx, result) {
 if (result.rows.length === 0) {
 //No Such Table Exists
 deferred.resolve("Success No Table");
 } else {
 var dropStatementString = "DROP TABLE " + tableName + ";";
 try {
 var request = _db.instance.transaction(function(tx) {
 tx.executeSql(dropStatementString, [],
 function(tx) {
 deferred.resolve("Success"); //now it is ready to be queried against
 });
 }, function (tx, error) {
 _db.utilityMethods.logErrors(error);
 deferred.reject(new Error(error));
 });
 } catch (e) {
 deferred.reject(new Error(e));
 }
 }
 });
 });
 } catch (error) {
 deferred.reject(new Error(e));
 }
 return deferred.promise;
 },
 /// <summary>
 /// Drop Entire Array of Tables
 /// </summary>
 /// <parameters>
 /// The Table Names
 /// </parameters>
 dropTableArray: function(tableNames) {
 var deferred = q.defer();
 var arrayLength = tableNames.length;
 for (var i = 0; i < arrayLength; i++) {
 try {
 var tableName = tableNames[i];
 var dropStatement = "DROP TABLE IF EXISTS " + tableName + ";";
 var request = _db.instance.transaction(function(tx) {
 tx.executeSql(dropStatement, [],
 function(tx) {
 if (i === (arrayLength - 1)) {
 deferred.resolve("Success");
 }
 }, function (tx, error) {
 _db.utilityMethods.logErrors(error);
 deferred.reject(new Error(error));
 });
 });
 } catch (e) {
 deferred.reject(new Error(e));
 }
 }
 return deferred.promise;
 },
 /// <summary>
 /// Drop Table then recreate it Factory method
 /// </summary>
 /// <parameters>
 /// The Table Name
 /// </parameters>
 /// <comment>
 /// Use this method as a hard reset especially after fetching new set of 
 /// data from server without dropping the entire database
 /// This resets the sequence of IDs as well
 /// </comment>
 purgeTable: function(tableName) {
 var deferred = q.defer();
 // Build a create statement based on tablename supplied
 // This will only work when a create statement is 
 // available in the sql dictionary
 function searchObj(query) {
 for (var key in window.sqlDictionary.createStatements) {
 //Forcing enumeration of prototype properties
 var value = window.sqlDictionary.createStatements[key];
 if (key === query) {
 return value;
 }
 }
 };
 var createStatement = searchObj(tableName);
 var existsQuery = "SELECT * FROM sqlite_master WHERE name = '" + tableName + "' and type='table'";
 try {
 var checkTable = _db.instance.transaction(function(tx) {
 tx.executeSql(existsQuery, [], function(tx, result) {
 if (result.rows.length === 0) {
 //No Such Table Exists
 window.dbService.createTable(createStatement)
 .then(function() {
 deferred.resolve("Success No Table");
 });
 } else {
 var dropStatement = "DROP TABLE " + tableName + ";";
 try {
 var request = _db.instance.transaction(function(tx) {
 tx.executeSql(dropStatement, [],
 function(tx) {
 window.dbService.createTable(createStatement)
 .then(function() {
 deferred.resolve("Success Table was dropped and Recreated");
 }); //now it is ready to be queried against
 });
 }, function (tx, error) {
 _db.utilityMethods.logErrors(error);
 deferred.reject(new Error(error));
 });
 } catch (e) {
 deferred.reject(new Error(e));
 }
 }
 });
 });
 } catch (error) {
 deferred.reject(new Error(e));
 }
 return deferred.promise;
 }
 };
 window.dbService = window.dbService || {};
 window.dbService.initDatabase = _db.initDatabase;
 window.dbService.open = _db.open;
 window.dbService.checkDbExists = _db.checkDbExists;
 window.dbService.createTable = _db.createTable;
 window.dbService.updateTable = _db.updateTable;
 window.dbService.insertTable = _db.insertTable;
 window.dbService.insertTableNoPromise = _db.insertTableNoPromise;
 window.dbService.insertTableArray = _db.insertTableArray;
 window.dbService.selectTable = _db.selectTable;
 window.dbService.selectCount = _db.selectCount;
 window.dbService.deleteAllRows = _db.deleteAllRows;
 window.dbService.dropTable = _db.dropTable;
 window.dbService.dropTableArray = _db.dropTableArray;
 window.dbService.purgeTable = _db.purgeTable;
}(window, Q, jQuery, cordova));

The factory methods are called like this :

Select:

dbServ.initDatabase().then(function (message) {
 dbServ.open(constants.databaseModel).then(function (db) {
 dbServ.selectTable(sqlDict.selectStatements.deficiencyCodes, "", "deficiencyTypCodes").then(
 function (data) {... do stuff}

dbServ is an alias for above class, and sqlDict is an object/dictionary of sql statements.

Insert:

 dbServ.insertTable(sqlDict.insertStatements.deficiency, {deficiencyObject}

A full implementation of a select will be like this:

loadCdTables: function () {
 var deferred = q.defer();
 //Aliases
 var sqlDict = window.sqlDictionary;
 var dbServ = window.dbService;
 var constants = app.constants;
 //Deficiency Codes
 dbServ.initDatabase().then(function (message) {
 dbServ.open(constants.databaseModel).then(function (db) {
 dbServ.selectTable(sqlDict.selectStatements.deficiencyCodes, "", "deficiencyTypCodes").then(
 function (data) {
 //do stuff with data
 deferred.resolve("Success");
 });
 });
 });
 return deferred.promise;
 },

Any ideas or inputs will be much appreciated.

chicks
2,8593 gold badges18 silver badges30 bronze badges
asked Aug 5, 2016 at 19:51
\$\endgroup\$
2
  • 1
    \$\begingroup\$ In your implementation .then nesting produces callback hell. It'd be nice to rework the code to have a typical straight Promise chain. \$\endgroup\$ Commented Aug 6, 2016 at 20:47
  • \$\begingroup\$ Could you give a simple example ? \$\endgroup\$ Commented Aug 6, 2016 at 21:46

1 Answer 1

3
\$\begingroup\$
dbServ.initDatabase().then(function (message) {
 dbServ.open(constants.databaseModel).then(function (db) {
 dbServ.selectTable(sqlDict.selectStatements.deficiencyCodes, "", "deficiencyTypCodes").then(
 function (data) {... do stuff}

This defeats the purpose of promises. Promises will still use callbacks, but with a twist. If the callback returns a Promise, the next then will not fire its callback until it resolves or rejects. This code can be written like the following, which looks fairly linear.

dbServ.initDatabase().then(function (message) {
 return dbServ.open(constants.databaseModel);
}).then(function(db){
 return dbServ.selectTable(sqlDict.selectStatements.deficiencyCodes, "", "deficiencyTypCodes");
}).then(function(data){
 // Do stuff
});

 initDatabase: function() {
 var deferred = q.defer();
 try {
 if (!window.openDatabase) // Check device supports SQLite or not.
 {
 deferred.reject(new Error("Databases are not supported in this device."));
 } else {
 setTimeout(function() { deferred.resolve("Database Created Successfully"); }, 100);
 }
 } catch (e) {
 if (e === 2) {
 // Version number mismatch. 
 deferred.reject(new Error("Invalid Version of Database"));
 } else {
 deferred.reject(new Error(e));
 }
 }
 return deferred.promise;
 },

Most browsers now support the native Promise. If not, you can easily use a polyfill. This makes your code depend on one less library, while allowing you to write promises natively. Should all browsers sufficiently support promises, you can take out the polyfill and you don't even need to rewrite your code.

initDatabase: function() {
 return new Promise(function(resolve, reject){
 if(!window.openDatabase) reject(...);
 else resolve(...);
 });
}

Also, the try-catch is useless in this case. There is no way the code in try can fail, unless deferred isn't an object, or deferred.reject isn't a function, or setTimeout isn't a function, all of which are impossible.

You also seem to use try-catch for async operations. try-catch does not capture async errors. However, promises treat unhandled errors as a reject, which will fire the error handler of the listening then.

In some JS engines, code inside a try-catch is not optimized. It's best to avoid wrapping code in try-catch and instead define the code in a function and call that function from the try-catch.


 //Step 2 Remove properties from the array that are not needed (updated date for example)
 //This is based on the Filter Array and needed for insert statement to work
 _db.instance.transaction(function(tx) {
 var transformedRecords = dataArray.map(dataArray => filterArray.reduce((p, c) => {
 p[c] = dataArray[c];
 return p;
 }, {}));
 //Saving the array in the db
 var saveData = function(dataArray, k) {
 //Step 3 Select and map individual records 
 var array = $.map(transformedRecords[k], function(value, index) {
 return [value];
 });

Where you actually need to use promises, you did not. I would suggest taking a look at Bluebird's promisify function. It converts a non-promise function into a promise function, if it follows the Node.js convention of async operations (params first, callback last, error object in callback first). That way, you can write this function like:

_db.instance.transaction().then(function(tx){
 ...
 return tx.executeSql(insertStatement, array)
}).then(function(tx){
 ...
});

Making a non-promise function a promise function manually is also simple, but tedious.

yourPromiseAsyncOperation: function(...args){
 return new Promise(function(resolve, reject){
 yourNonPromiseAsyncOperation(...args, function(error, data){
 if(error) reject(error);
 else resolve(data);
 })
 });
}

var existsQuery = "SELECT * FROM sqlite_master WHERE name = '" + tableName + "' and type='table'";

You should really consider sanitation of your queries.

answered Aug 7, 2016 at 1:47
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.