patternjavascriptMinor
Set of handlers for commonly used functionalities
Viewed 0 times
handlersfunctionalitiesusedforsetcommonly
Problem
I am developing a set of simple JS functions which can be used for most commonly required JS functionalities. I want the functions to be very easy to use for new JS developers.
Is the code structured well? How can it be improved?
```
"use strict";
/ getElement, elementBase must be included /
var littleJS = function() {
var elementNotFound = function() { console.log('No element found'); }
var little = {
// Override this function to execute statements when an ajax call is made
'startAjaxFunc' : null,
// Override this function to execute statements when an ajax call gets over
'stopAjaxFunc' : null,
/* To make ajax call. First parameter is JSON of async (true or false), method (default GET), returnType (default JSON), data to be sent and url to be hit.
Second parameter is callback function that will be called on success */
'makeAjaxCall' : function(args, callback) {
if (!args.async)
args.async = true;
if (!args.method)
args.method = "GET";
if (!args.returnType)
args.returnType = "JSON";
if (!args.data)
args.data = {};
var JSONToURLString = function(obj) {
var urlPara = '';
for ( var key in obj) {
urlPara = urlPara + key + '=' + obj[key] + '&';
}
return urlPara.substring(0, urlPara.length - 1);
};
args.data = JSONToURLString(args.data);
if (args.method === "GET")
args.url = args.url + '?' + args.data;
var xmlhttp;
if (window.XMLHttpRequest) {// code for IE7+, Firefox, Chrome,
// Opera, Safari
xmlhttp = new XMLHttpRequest();
} else {// code for IE6, IE5
xmlhttp = new ActiveXObject("Microsoft.XMLHTTP");
}
xmlhttp.onreadystatechange = function() {
Is the code structured well? How can it be improved?
```
"use strict";
/ getElement, elementBase must be included /
var littleJS = function() {
var elementNotFound = function() { console.log('No element found'); }
var little = {
// Override this function to execute statements when an ajax call is made
'startAjaxFunc' : null,
// Override this function to execute statements when an ajax call gets over
'stopAjaxFunc' : null,
/* To make ajax call. First parameter is JSON of async (true or false), method (default GET), returnType (default JSON), data to be sent and url to be hit.
Second parameter is callback function that will be called on success */
'makeAjaxCall' : function(args, callback) {
if (!args.async)
args.async = true;
if (!args.method)
args.method = "GET";
if (!args.returnType)
args.returnType = "JSON";
if (!args.data)
args.data = {};
var JSONToURLString = function(obj) {
var urlPara = '';
for ( var key in obj) {
urlPara = urlPara + key + '=' + obj[key] + '&';
}
return urlPara.substring(0, urlPara.length - 1);
};
args.data = JSONToURLString(args.data);
if (args.method === "GET")
args.url = args.url + '?' + args.data;
var xmlhttp;
if (window.XMLHttpRequest) {// code for IE7+, Firefox, Chrome,
// Opera, Safari
xmlhttp = new XMLHttpRequest();
} else {// code for IE6, IE5
xmlhttp = new ActiveXObject("Microsoft.XMLHTTP");
}
xmlhttp.onreadystatechange = function() {
Solution
Copy-paste your code into http://jshint.com/, it will point out many things you should fix. Try to fix them all.
When creating objects, you don't need to quote keys that are simple words.
For example, instead of this:
This writing style is simpler and more common:
When iterating over an object, you need to add an
For example, instead of this:
Write like this:
There are two problems with this:
I propose this approach instead:
Without jQuery, you could use a simple implementation of
Note: the fact that I modify
When creating objects, you don't need to quote keys that are simple words.
For example, instead of this:
var little = {
'startAjaxFunc' : null,
'stopAjaxFunc' : null,This writing style is simpler and more common:
var little = {
startAjaxFunc: null,
stopAjaxFunc: null,When iterating over an object, you need to add an
if statement to filter unwanted properties from the prototype.For example, instead of this:
for ( var key in obj) {
urlPara = urlPara + key + '=' + obj[key] + '&';
}Write like this:
for (var key in obj) {
if (!obj.hasOwnProperty(key)) {
continue;
}
urlPara = urlPara + key + '=' + obj[key] + '&';
}if (!args.async)
args.async = true;
if (!args.method)
args.method = "GET";
if (!args.returnType)
args.returnType = "JSON";
if (!args.data)
args.data = {};There are two problems with this:
- It modifies
args, which was passed as a parameter. It's a bad practice to modify parameters, unless they are intended to be out parameters in your API
- It's tedious
I propose this approach instead:
var initial = {
async: true,
method: "GET",
returnType: "JSON",
data: {}
};
var options = $.extend(initial, args);Without jQuery, you could use a simple implementation of
extend instead:function extend(base, params) {
for (var key in params) {
if (params.hasOwnProperty(key)) {
base[key] = params[key];
}
}
return base;
}Note: the fact that I modify
base may seem to contradict with my point earlier about not modifying parameter variables. In this case this is ok, because modifying base is a conscious move, it's the main purpose of the method.Code Snippets
var little = {
'startAjaxFunc' : null,
'stopAjaxFunc' : null,var little = {
startAjaxFunc: null,
stopAjaxFunc: null,for ( var key in obj) {
urlPara = urlPara + key + '=' + obj[key] + '&';
}for (var key in obj) {
if (!obj.hasOwnProperty(key)) {
continue;
}
urlPara = urlPara + key + '=' + obj[key] + '&';
}if (!args.async)
args.async = true;
if (!args.method)
args.method = "GET";
if (!args.returnType)
args.returnType = "JSON";
if (!args.data)
args.data = {};Context
StackExchange Code Review Q#67518, answer score: 3
Revisions (0)
No revisions yet.