Coding Tips - nuvolo/coding-style-guide GitHub Wiki
1 - Console logging hint
Normal approach
function successResponse(response){
//Response is an JSON object
console.log("Response is: " + JSON.stringify(response));
}
Recommended Approach
function successResponse(response){
//Response is an JSON object
console.log("Response is: ", response);
}
- It might possible that the response payload would not be a proper JSON
- While using JSON.stringify() the whole dataset would become a plain string which is difficult to read
- With the help of recommended approach it would it so easy to traverse the whole structure
2 - Checking of value and its data
Normal Approach
function processSyncIssues(payload){
var thisDbAction = payload['db_action'];
if (thisDbAction === 'delete') {
...
}
}
Recommended Approach
function processSyncIssues(payload){
var thisDbAction = payload['db_action'];
if (thisDbAction && thisDbAction === 'delete') {
...
}
}
- It might possible that the response payload['db_action'] returns and undefined hence it's not a good practise to avoid thisDbAction valid skip runtime expectations
- Via implementing following approach we're explicitly making our condition ever more stronger than before
3 - Conditional statements
Normal Approach
function processSyncIssues(payload){
var isValidObject = hasKey(thisErr, 'table');
if(!isValidObject){
return;
} else {
...
}
}
Recommended Approach
function processSyncIssues(payload){
var isValidObject = hasKey(thisErr, 'table');
if(!isValidObject){
return;
}
...
}
- Since IF statement is already handling its condition hence instead of wiring else , we can simply skip it.
- It'll let our code be more clear and visible for debugging
4 - Injecting new logic or code
Normal Approach
function initializeView(payload){
...
var fromRecoverKey = $location.search().parm1 || false;
var fromRecoverValue = $location.search().parm2 || false;
//Whole recover logic while preparing form which includes
following dependencies like MobiusDb_records etc.
...
}
Recommended Approach
function initializeView(payload){
...
//Introduced a middleware known as $recoverProvider service
$recoveryProvider.checkAndPrepareRecoveryOfData().then(function(dataset){
...
}
...
}
- Moved whole new logic to another service known as recoveryProvider and it also helped us to remove MobiusDB_records dependency from view.js controller.
- Direct database connectivity is no longer available now, hence the view.js controller is not tightly coupled with Database services but connected through a middleware service.
- No direct logic is implemented under view.js for preparing recovery of form, but the fetching of all data with sync_queue data will handled under recoverProvider service
5 - Avoid using dependencies directly
Normal Approach
# sync.js
function step5A(key,appId,viewId){
...
ViewProvider.openView(reqParams);
}
Recommended Approach
# sync.js
function step5A(key,appId,viewId){
...
$recoverProvider.openView(reqParams);
}
- The ViewProvider is a view controller which should not be directly called from a service.
- Since its related to recovery of data hence instead of calling directly we'll call it through $recoverProvider service
6 - Adding new logic to function
Normal Approach
# sync.js
function addToQueue(d,forceSync) {
...
var batchId = getSyncBatchId();
if (batchId == "") {
var refreshSyncBatch = refreshSyncBatchId();
if (refreshSyncBatch != "") {
d.__MSYNC_BATCH_ID__ = refreshSyncBatchId;
}
} else {
d.__MSYNC_BATCH_ID__ = batchId;
}
...
...
...
}
Recommended Approach
# sync.js
function addToQueue(d,forceSync) {
/**
Avoid mutating queue form params under 'ms.js' but handle under 'sync.js'
Assign __MSYNC_BATCH_ID__ here using localStorage instead of calling
DB service again. In following logic use following approach under sync.js
*/
d.__MSYNC_BATCH_ID__ = getSyncBatchId();
...
...
...
}
- Avoid using checks and validation inside caller function , make sure the called function should return response accordingly and must be handled all fallback.
- Since the caller function should never know the logic of called function, i.e it should use the response directly
7 - Introducing new parameter to MobiusForm required by sync.js
Normal Approach
# sync.js file
function MobiusRecord(table) {
this.__MSYNC_BATCH_ID__ = "";
...
}
MobiusRecord.prototype.setSyncBatchId = function(v) {
/**
This process will check for existing sync_record and then fetch
__MSYNC_BATCH_ID__ from payload and if not found it will generate
a new one and assign it to 'this.__MSYNC_BATCH_ID__'
*/
...
}
MobiusRecord.prototype.getSyncBatchId = function() {
return this.this.__MSYNC_BATCH_ID__;
}
Recommended Approach
# sync.js
function addToQueue(d,forceSync) {
/**
Avoid mutating queue form params under 'ms.js' but handle under 'sync.js'
Assign __MSYNC_BATCH_ID__ here using localStorage instead of calling
DB service again. In following logic use following approach under sync.js
*/
d.__MSYNC_BATCH_ID__ = getSyncBatchId();
...
...
...
}
function refreshSyncBatchId() {
var batchId = _generateBatchId();
localStorage.setItem('__MSYNC_BATCH_ID__', batchId);
return batchId;
}
function clearSyncBatchId() {
localStorage.removeItem("__MSYNC_BATCH_ID__");
return;
}
function getSyncBatchId() {
var batchId = localStorage.getItem('__MSYNC_BATCH_ID__') || "";
if (batchId == "") {
return refreshSyncBatchId();
}
return batchId;
}
- Avoid mutating object into parent file if they're not related to them.
- Avoid using DB calls again and again and start using localStorage approach and reset it once sync gets completed.
8 - Clearing Sync queue and remove from_recover param
Normal Approach
# sync.js
function removeAndSyncErrorQueueRecord(key) {
if(!key){
return;
}
clearRecoveryParam();
return MobiusDB_Records.removeQueue("id",key)
.then(
getQueueData,
getQueueData,
{},{}
);
}
function clearRecoveryParam () {
var from_recover = $location.search().from_recover || false;
if(!from_recover){
return;
}
$location.search('from_recover', null);
return;
}
Recommended Approach
# Sync.js
function removeAndSyncErrorQueueRecord(key) {
if(!key){
return;
}
return MobiusDB_Records.removeQueue("id",key)
.then(clearRecoveryParam)
.then(
getQueueData,
getQueueData,
{},{}
);
}
function clearRecoveryParam () {
var from_recover = $location.search().from_recover || false;
if(!from_recover){
return;
}
$location.search('from_recover', null);
return;
}
- While clearing the from_Recover from URL it should be handled in such a manner that is should only be clean if record get cleared from the queue. Hence using under promise with .then() function would be a neat and clean approach to execute