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

Developing a JavaScript library

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
javascriptlibrarydeveloping

Problem

I am slowly learning to master JavaScript and particularly the art of self-executing/invoking functions. I have developed a simple JavaScript plugin and I think I have followed the correct standards.

It is pretty basic in what it does, it is to store information on a oData BATCH request, which provides functions to add to the batch request, get the string representation (which can then be used in a HTTP request), and I am expanding on it as we speak.

However, the code is as follows. It was using jQuery but not any more due to resource constraint.

I am generally after comments on what could have been done better. I am doing this in an attempt to reduce the window namespace. Therefore BatchInfo and BatchCacheRequest (which are a bit more specific to my code) are not clogging up the name space, nor do I have to keep an array anywhere do hold the BatchInfo objects.

```
(function () {
var info = [],
b = {
clear: function () {
info = [];
},

get: function () {
return info;
},

add: function (command, url, data) {
info.push(new BatchInfo(command, url, data));
return info;
},

toString: function () {
var batchString = ['--batchfull'];

batchString.push('Content-Type: multipart/mixed; boundary=batchitems');

for (var i = 0; i < info.length; i++) {
batchString.push('');
batchString.push('--batchitems');
batchString.push('Content-Type: application/http');
batchString.push('Content-Transfer-Encoding: binary');
batchString.push('');
batchString.push(info[i].Command + ' ' + info[i].URL + ' HTTP/1.1');

if (info[i].Command !== 'DELETE') {
batchString.push('Content-Type: application/json;type=entry');

Solution

Not "stupidly bad", but you're invoking .push() far more times than needed. It's a variadic function, so group the items into single invocations.

Additionally, I'd probably put a toString() method on the BatchInfo.prototype that will give back the string representation of the object.

Then put the objects directly into what was the batchString array. That way the .toString() method will be invoked when you call .join().

I've included additional small changes below as well.

(function () {
    var info = [],
        b = {
            clear: function () {
                info.length = 0; // Clear all references
            },
            get: function () {
                return info;
            },
            add: function (command, url, data) {
                info.push(new BatchInfo(command, url, data));
                return info;
            },
            toString: function () {
                // Put the `BatchInfo` objects directly into the Array.
                // Now we don't need the `for` loop.
                return ['--batchfull',
                        'Content-Type: multipart/mixed; boundary=batchitems']
                           .concat(info).join('\r\n'); // Not needed-- || '';
            }
        };
    function BatchInfo(command, url, data) {
        this.URL = url || '';
        this.Command = command || '';
        this.Data = data || '';
    }
    // Handle the `toString()` representation here
    BatchInfo.prototype.toString = function() {
        var parts = ['', '--batchitems',
                     'Content-Type: application/http',
                     'Content-Transfer-Encoding: binary',
                     '',  this.Command + ' ' + this.URL + ' HTTP/1.1'];

        if (this.Command !== 'DELETE') {
            parts.push('Content-Type: application/json;type=entry');
        }
        parts.push('', this.Data);
        return parts.join('\r\n');
    };
    function BatchCacheRequest(info, description) {
        this.Info = info || '';
        this.Description = description || '';
        this.SubmitDateTime = new Date().toString('yyyy-MM-dd HH:mm');
    }
    window.batch = b;
})();

Code Snippets

(function () {
    var info = [],
        b = {
            clear: function () {
                info.length = 0; // Clear all references
            },
            get: function () {
                return info;
            },
            add: function (command, url, data) {
                info.push(new BatchInfo(command, url, data));
                return info;
            },
            toString: function () {
                // Put the `BatchInfo` objects directly into the Array.
                // Now we don't need the `for` loop.
                return ['--batchfull',
                        'Content-Type: multipart/mixed; boundary=batchitems']
                           .concat(info).join('\r\n'); // Not needed-- || '';
            }
        };
    function BatchInfo(command, url, data) {
        this.URL = url || '';
        this.Command = command || '';
        this.Data = data || '';
    }
    // Handle the `toString()` representation here
    BatchInfo.prototype.toString = function() {
        var parts = ['', '--batchitems',
                     'Content-Type: application/http',
                     'Content-Transfer-Encoding: binary',
                     '',  this.Command + ' ' + this.URL + ' HTTP/1.1'];

        if (this.Command !== 'DELETE') {
            parts.push('Content-Type: application/json;type=entry');
        }
        parts.push('', this.Data);
        return parts.join('\r\n');
    };
    function BatchCacheRequest(info, description) {
        this.Info = info || '';
        this.Description = description || '';
        this.SubmitDateTime = new Date().toString('yyyy-MM-dd HH:mm');
    }
    window.batch = b;
})();

Context

StackExchange Code Review Q#35756, answer score: 5

Revisions (0)

No revisions yet.