3
\$\begingroup\$

I am sure there are better ways to do things but this is part of my first JavaScript project and I'm slowly getting accustomed with how things work in JavaScript. This is also why I ask for a code review, to find what I could not see.

Suggestions on idiomatic JavaScript would be welcome.

/* Circular buffer used to add values in blocks of constant size */
function IndexOutOfBounds(maxIndex, requestedIndex, method) {

  this.maxIndex       = maxIndex;
  this.requestedIndex = requestedIndex;
  this.method         = method;

  this.toString = function () {
    return "Invalid index in method ['" + this.method + "']\n" +
           "Requested index    : " + this.requestedIndex + "\n" +
           "Valid _buffer index : " + "[0.." + this.maxIndex + "]";
  };
}

function ImproperBlockLength(ringBlockLength, givenBlockLength) {
  this.ringBlockLength  = ringBlockLength;
  this.givenBlockLength = givenBlockLength;

  this.toString = function () {
    return "Block length mismatch.\n" +
           "Requeste block length : " + this.givenBlockLength  + "\n" +
           "Valid block length    : " + this.ringBlockLength;
  };
}

function Ring(length, blockLength) {
  this.length    = length;
  this._maxIndex = this.length - 1;
  this._start    = 0;
  this._buffer   = new Float32Array(this.length);

  /* blockLength should always be a factor of size.
   * An exception is thrown if it's not;
   */
  this._blockLength = 0;

  if (blockLength) {

    if (length % blockLength != 0) {
      throw "Block length must be a factor of length.";
    } else {
      this._blockLength = blockLength;
    }

  }
}

Ring.prototype.checkBounds = function (requested, callerName) {

  if (requested < 0 || requested > this._maxIndex)
    throw new IndexOutOfBounds(this._maxIndex, requested, callerName);
};

Ring.prototype.relativeIndex = function(index) {
  return (this._start + index) % this.length;
};

/* Should not be used when there is a set _blockLength */
Ring.prototype.push = function (element) {

  this._buffer[this._start] = element;

  var newStart = this._start + 1;
  this._start = newStart > this._maxIndex ? 0 : newStart;
};

Ring.prototype.get = function (index) {

  this.checkBounds(index, 'get');

  return this._buffer[this.relativeIndex(index)];
};

Ring.prototype.set = function (index, value) {

  this.checkBounds(index, 'set');

  this._buffer[this.relativeIndex(index)] = value;

};

Ring.prototype.concat = function(arr) {

  var alen = arr.length;
  var blen = this._blockLength;

  if (alen != blen) {
    throw new ImproperBlockLength(blen, alen);
  }

  this._buffer.set(arr, this._start);
  this._start = (this._start + alen) % this.length;

};

Ring.prototype.map = function (callback) {

  var relativeIndex;
  var value;

  for(var i = 0 ; i < this.length ; i++) {
    relativeIndex = this.relativeIndex(i);
    value = this._buffer[relativeIndex];

    this._buffer[relativeIndex] = callback(value, i, this.length);
  }
};
\$\endgroup\$
  • \$\begingroup\$ You could move your this.toString methods out from their respective functions and make them xxx.prototype.toString and thereby they are not created on every new instance but reused from the prototype. I am not a fan of the x ? y : z conditionals, personally I prefer the long if .. then .. else for readability (but this is just personal preference). I also prefer i += 1; to i++. You could run your code through jslint for further styling tips and it may find some errors/warnings that you haven't considered. \$\endgroup\$ – Xotic750 Jul 5 '13 at 10:38
1
\$\begingroup\$

I haven't examined the actual functionality, but this is based on my comment about moving this.toString to a prototype method (which is, I feel, the main improvement) and then I've done a few micro optimisations (but nothing that is going to make any real noticeable difference). Other than that I performed a bit of personal styling, there was nothing particularly wrong with what you had.

Javascript

/*jslint maxerr: 50, indent: 4, browser: true, nomen: true */
/*global Float32Array */

(function () {
    "use strict";

    /* Circular buffer used to add values in blocks of constant size */

    function IndexOutOfBounds(maxIndex, requestedIndex, method) {
        this.maxIndex = maxIndex;
        this.requestedIndex = requestedIndex;
        this.method = method;
    }

    IndexOutOfBounds.prototype.toString = function () {
        return "Invalid index in method ['" + this.method + "']\nRequested index    : " + this.requestedIndex + "\nValid _buffer index : [0.." + this.maxIndex + "]";
    };

    function ImproperBlockLength(ringBlockLength, givenBlockLength) {
        this.ringBlockLength = ringBlockLength;
        this.givenBlockLength = givenBlockLength;
    }

    ImproperBlockLength.prototype.toString = function () {
        return "Block length mismatch.\nRequeste block length : " + this.givenBlockLength + "\nValid block length    : " + this.ringBlockLength;
    };

    function Ring(length, blockLength) {
        this.length = length;
        this._maxIndex = length - 1;
        this._start = 0;
        this._buffer = new Float32Array(length);

        /* blockLength should always be a factor of size.
         * An exception is thrown if it's not;
         */
        this._blockLength = 0;
        if (blockLength) {
            if (length % blockLength !== 0) {
                throw "Block length must be a factor of length.";
            }

            this._blockLength = blockLength;
        }
    }

    Ring.prototype.checkBounds = function (requested, callerName) {
        var maxIndex = this._maxIndex;

        if (requested < 0 || requested > maxIndex) {
            throw new IndexOutOfBounds(maxIndex, requested, callerName);
        }
    };

    Ring.prototype.relativeIndex = function (index) {
        return (this._start + index) % this.length;
    };

    /* Should not be used when there is a set _blockLength */
    Ring.prototype.push = function (element) {
        var start = this._start,
            newStart = start + 1;

        this._buffer[start] = element;
        this._start = 0;
        if (newStart <= this._maxIndex) {
            this._start = newStart;
        }
    };

    Ring.prototype.get = function (index) {
        this.checkBounds(index, 'get');

        return this._buffer[this.relativeIndex(index)];
    };

    Ring.prototype.set = function (index, value) {
        this.checkBounds(index, 'set');
        this._buffer[this.relativeIndex(index)] = value;

    };

    Ring.prototype.concat = function (arr) {
        var alen = arr.length,
            blen = this._blockLength,
            start;

        if (alen !== blen) {
            throw new ImproperBlockLength(blen, alen);
        }

        start = this._start;
        this._buffer.set(arr, start);
        this._start = (start + alen) % this.length;

    };

    Ring.prototype.map = function (callback) {
        var relativeIndex,
            length = this.length,
            i = 0;

        while (i < length) {
            relativeIndex = this.relativeIndex(i);
            this._buffer[relativeIndex] = callback(this._buffer[relativeIndex], i, length);
            i += 1;
        }
    };
}());

One other thing to consider is, because you will only be using newer browsers that support Float32Array then you could also define properties using Object.defineProperty

Update: Here is a using Object.defineProperty, I did it kind of quick so there are no guarantees that I did it correctly, it is more to give you a starter, hope it helps.

Javascript

/*jslint maxerr: 50, indent: 4, browser: true, nomen: true, bitwise: true */
/*global console, Float32Array */

(function () {
    "use strict";

    var Ring = (function () {
        /* Circular buffer used to add values in blocks of constant size */

        var minIndex = 0,
            maxIndex = 4294967294;

        function clamp(number, min, max) {
            return Math.min(Math.max(number, min), max);
        }

        function IndexOutOfBounds(maxIndex, requestedIndex, method) {
            Object.defineProperties(this, {
                "maxIndex": {
                    value: maxIndex
                },

                "requestedIndex": {
                    value: requestedIndex
                },

                "method": {
                    value: method
                }
            });
        }

        Object.defineProperty(IndexOutOfBounds.prototype, "toString", {
            get: function () {
                return "Invalid index in method ['" + this.method + "']\nRequested index    : " + this.requestedIndex + "\nValid _buffer index : [0.." + this.maxIndex + "]";
            }
        });

        function ImproperBlockLength(ringBlockLength, givenBlockLength) {
            Object.defineProperties(this, {
                "ringBlockLength": {
                    value: ringBlockLength
                },

                "givenBlockLength": {
                    value: givenBlockLength
                }
            });
        }

        Object.defineProperty(ImproperBlockLength.prototype, "toString", {
            get: function () {
                return "Block length mismatch.\nRequeste block length : " + this.givenBlockLength + "\nValid block length    : " + this.ringBlockLength;
            }
        });

        function Ring(length, blockLength) {
            length = clamp(length, minIndex, maxIndex + 1) >>> 0;

            /* blockLength should always be a factor of size.
             * An exception is thrown if it's not;
             */
            blockLength = clamp(blockLength, minIndex, maxIndex + 1) >>> 0;
            if (length % blockLength !== 0) {
                throw "Block length must be a factor of length.";
            }

            var privateStart = 0,
                privateBuffer = new Float32Array(length);

            Object.defineProperties(this, {
                "length": {
                    get: function () {
                        return length;
                    },

                    set: function (newLength) {
                        length = clamp(newLength, minIndex, maxIndex + 1) >>> 0;
                    }
                },

                "_start": {
                    get: function () {
                        return privateStart;
                    },

                    set: function (start) {
                        privateStart = clamp(start, minIndex, maxIndex) >>> 0;
                    }
                },

                "_buffer": {
                    get: function () {
                        return privateBuffer;
                    },

                    set: function (buffer) {
                        privateBuffer = buffer;
                    }
                },

                "_blockLength": {
                    value: blockLength
                }
            });
        }

        Object.defineProperties(Ring.prototype, {
            "_maxIndex": {
                value: function () {
                    return clamp(this.length - 1, minIndex, maxIndex) >>> 0;
                }
            },

            "checkBounds": {
                value: function (requested, callerName) {
                    var maxIndex = this._maxIndex;

                    if (requested < 0 || requested > maxIndex) {
                        throw new IndexOutOfBounds(maxIndex, requested, callerName);
                    }

                    return maxIndex;
                }
            },

            "relativeIndex": {
                value: function (index) {
                    return (this._start + index) % this.length;
                }
            },

            /* Should not be used when there is a set _blockLength */
            "push": {
                value: function (element) {
                    var start = this._start,
                        newStart = start + 1;

                    this.buffer = element;
                    if (newStart > this._maxIndex) {
                        this._start = 0;
                    } else {
                        this._start = newStart;
                    }
                }
            },

            "get": {
                value: function (index) {
                    this.checkBounds(index, 'get');

                    return this._buffer[this.relativeIndex(index)];
                }
            },

            "set": {
                value: function (index, value) {
                    this.checkBounds(index, 'set');
                    this._buffer[this.relativeIndex(index)] = value;
                }
            },

            "concat": {
                value: function (arr) {
                    var alen = arr.length,
                        blen = this._blockLength,
                        start;

                    if (alen !== blen) {
                        throw new ImproperBlockLength(blen, alen);
                    }

                    start = this._start;
                    this._buffer.set(arr, start);
                    this._start = (start + alen) % this.length;
                }
            },

            "map": {
                value: function (callback) {
                    var relativeIndex,
                        length = this.length,
                        i = 0;

                    while (i < length) {
                        relativeIndex = this.relativeIndex(i);
                        this._buffer[relativeIndex] = callback(this._buffer[relativeIndex], i, length);
                        i += 1;
                    }
                }
            }
        });

        return Ring;
    }());

    var ring = new Ring(10, 1);

    console.log({
        0: ring,
        1: ring.length
    });
}());

On jsfiddle

\$\endgroup\$
  • \$\begingroup\$ Thank you for your tips, i just found out about Object.defineProperty a few days ago and i was considering using it since it gives me the oportunity to set the writable descriptor on object properties. I'm not very keen of the ternary operator :? either but in that one case i think it improves readability while also saving a bit of typing. \$\endgroup\$ – user1105045 Jul 5 '13 at 12:45
  • \$\begingroup\$ No problem, I have added a quick conversion that will hopefully give you a starter. Good luck. \$\endgroup\$ – Xotic750 Jul 5 '13 at 21:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.