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

Set of handlers for commonly used functionalities

Submitted by: @import:stackexchange-codereview··
0
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() {

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:

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.