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

Cross-browser DOMTokenList object and wrapper function. Failures and improvements?

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

Problem

I have been looking for a Cross-browser solution for DOMTokenList and element.classList. I wasn't able to find much for DOMTokenList and the polyfills that I found for element.classList were mostly limited to IE8+ as they relied on modifying the DOM Element prototype. There are of course some other stand alone functions/methods that perform element.classList, as can be found in Change an element's CSS class with JavaScript. Anyway, I thought I'd create my own solution and as generically as possible. The code that follows is working, but unfortunately I am unable to test on any Microsoft browsers. So, I'm looking for any edge cases where the code may fail or suggestions for improvement, before I make it publicly available on GitHub or GIST (probably GIST would be best for such a project?).

Javascript

```
/jslint maxerr: 50, indent: 4, browser: true, bitwise: true, white: true /

/* DOMTokenList v1.2
*
* home: http://code.google.com/p/domtokenlist//
*
* This type represents a set of space-separated tokens.
* Commonly returned by HTMLElement.classList, HTMLLinkElement.relList,
* HTMLAnchorElement.relList or HTMLAreaElement.relList.
* It is indexed beginning with 0 as with JavaScript arrays.
* DOMTokenList is always case-sensitive. Written with cross-browser compatibility in mind and
* does not require any external libraries.
*/

var tokenList = (function (undef) {
"use strict";

/* The space characters, for the purposes of this specification,
* are U+0020 SPACE, U+0009 CHARACTER TABULATION (tab), U+000A LINE FEED (LF),
* U+000C FORM FEED (FF), and U+000D CARRIAGE RETURN (CR).
*
* The White_Space characters are those that have the Unicode property "White_Space"
* in the Unicode PropList.txt data file.
*
* http://www.whatwg.org/specs/web-apps/current-work/multipage/common-microsyntaxes.html#space-character
*/
var whiteSpaces = " \\t\\n\\f\\r",
wsSplitRX = new RegExp("[" + whiteSpaces + "]

Solution

From top to bottom, my 2 cents:

function isValid(object) {
  return object !== undef && object !== null;
}


object parameter should be more aptly named ( value? ) since you will check non objects with this that are valid.

if (!isValid(string)) {
  string = toStringFn.call(string);
}


Seems wrong, why not

if( typeof string === "function" ){
  string = toStringFn.call(string);
}


You error out in both errorIfEmpty and validTokenString, but only the first function mentions that in the function name. I would expect then that validTokenString would not throw an error, and that it would return a boolean, not the the string I passed.

4294967294 is magic number, you should at least comment how you got there

Same goes for isValidIndexRange, functions with isXXX should return booleans, and not throw errors.

4294967295 is a magic number as well. isValidLength is not consistent with they way you wrote isValidIndex and isValidIndexRange.

function push() is odd, why do you not use the built-in push of Array after you check the validIndexRange? If there is a difference, you should comment it.

for indexOf, I would first check whether the browser has it built in, and use that if possible. Also consider something like

function indexOf(array, searchElement) {
    var pointer = array.length;

    while (pointer--) {
        if (searchElement === array[pointer]) {
            return pointer;
        }
    }
    return -1;
}


Personally, for short functions, I tend to drop the curlies:

function indexOf(array, searchElement)
{
    var pointer = array.length;

    while (pointer--)
      if (searchElement === array[pointer])
        return pointer;

    return -1;
}


Trinary could help for elementToString

function elementToString(element) {
    return isValid(element)?element.toString():""
}


it could also help for determining k
k = (index < length)?index:length;

For loops could also make your code easier to parse, less sprinkled with housekeeping.

var i = 0;
this.update();
while (i < length) {
  DOMTokenList.prototype.remove.call(this, arguments[i]);
  this.setProperty();
  i += 1;
}


could be

this.update();
for (var i  = 0 ; i < length ;i++) {
  DOMTokenList.prototype.remove.call(this, arguments[i]);
  this.setProperty();
}


Having to call this.update(); in every function of TokenList is a code smell to me, I am not sure what to recommend instead, but it sure does not look good.

Finally, I think this could a tad more commenting ;)

Code Snippets

function isValid(object) {
  return object !== undef && object !== null;
}
if (!isValid(string)) {
  string = toStringFn.call(string);
}
if( typeof string === "function" ){
  string = toStringFn.call(string);
}
function indexOf(array, searchElement) {
    var pointer = array.length;

    while (pointer--) {
        if (searchElement === array[pointer]) {
            return pointer;
        }
    }
    return -1;
}
function indexOf(array, searchElement)
{
    var pointer = array.length;

    while (pointer--)
      if (searchElement === array[pointer])
        return pointer;

    return -1;
}

Context

StackExchange Code Review Q#27968, answer score: 4

Revisions (0)

No revisions yet.