Check for classname regex question

Started work last night on writing some dom ‘class’ functions. ‘addClass’ and ‘removeClass’ so far. Got a little carried away and wrapped them up in a basic Jquery-esque constructor/selector.

The class methods utitilise ‘classlist’ if available IE10 etc. and fall back on hand rolled code if not.

It needs a polyfill for IE8’s querySelectAll and it’s lack of pseudo selectors like nth-child etc.

My particular issue is with the addClass method. I’m using a regex to test if the class already exists. Albeit I’ve never used them myself, as per what I have read, it test for hyphens in the name. As javascript’s flavour of regex doesn’t support look behinds, I’m having to bodge it by stripping out lookbehind matches first before testing for matches with a look ahead.

The code snipped is here.

// Strips lookbehind matches before testing. Needs work!!
  preRx = new RegExp('[\\\\S-]' + clName,'g');
  classRx = new RegExp(clName + '(?![-\\\\S])');

  for (; i < len; i += 1) {
    if (!(classRx.test(els[i].className.replace(preRx, '')))) {
      els[i].className = els[i].className + ' ' + clName;
   }
}

I know Jquery pads out the classname string first, left and right and then uses a while loop and index of ’ ’ + className + ’ ’ to find a match.

Wondered if the regex above could be improved on or combined into one. Just feels a bit clunky.

Here’s the complete code with example usage.

<!DOCTYPE html>
<html lang="en">
<head> <meta charset="utf-8">
<title>Root QuerySelector</title>
<style>
li.odd { background-color: gray; }

li.even { background-color: teal!important; }

li {
  background-color: darkslategray;
}

.whiteText{ color: white; }
</style>
</head>
<body>
<ul id = 'list1'>
  <li>Item 1</li>
  <li>Item 2</li>
  <li>Item 3</li>
  <li>Item 4</li>
  <li>Item 5</li>
  <li>Item 6</li>
  <li>Item 7</li>
  <li>Item 8</li>
  <li>Item 9</li>
  <li>Item 10</li>
</ul>
<ul id = 'list2'>
  <li>Item 1</li>
  <li>Item 2</li>
  <li>Item 3</li>
  <li>Item 4</li>
  <li>Item 5</li>
  <li>Item 6</li>
  <li>Item 7</li>
  <li>Item 8</li>
  <li>Item 9</li>
  <li>Item 10</li>
</ul>

<button type="button" onclick="Dom('li:nth-child(even)').addClass('even')">Set class name 'even'</button>
<br>
<button type="button" onclick="Dom('li').removeClass('even')">Remove class name 'even'</button>
<script>
// ------------------------------------ Dom Select -----------------------------------
(function(win, doc){

  var docElem = doc.documentElement,

      slice = [].slice,
      push = [].push,
      toString = {}.toString,

      // native trim not supported in older versions of IE
      trim = function(strg) { return strg.replace(/^\\s+|\\s+$/g, '') },

      objType = function(obj){ return / ([^\\]]+)/.exec({}.toString.call(obj))[1]; },

      isNodeList = function(obj) {
        return /HTMLCollection|HTMLUListElement/.test(objType(obj));
      },

      //------------------- Shortcuts to default selectors. ------------------

      getById = function getById(id) { return doc.getElementById(id); },

      getByTag = function getByTag(name, root) { return (root||doc).getElementsByTagName(name); },

      // Needs polyfill for IE8 and pseudo selectors
      queryAll = function (name, root) { return (root||doc).querySelectorAll(name); },

      // Fallback for IE 8 use querySelectorAll instead.
      getByClass = (docElem.getElementsByClassName)
                   ? function getByClass(name, root) { return (root||doc).getElementsByClassName(name); }
                   : queryAll,

      //--------------------------- Class Methods -----------------------------

      //----------------------------- Add Class -------------------------------

      addClass = function (clName) {

        var i = 0, len, classRx, preRx, els = this;

        if(len = els.length) {

          if (clName = (typeof clName === 'string' && trim(clName))) {
            // Use native classList method if available. Not supported in IE8-9.
            if(els[0].classList)

              for (; i < len; i += 1) els[i].classList.add(clName);

            else {
              // Strips lookbehind matches before testing. Needs work!!
              preRx = new RegExp('[\\\\S-]' + clName,'g');
              classRx = new RegExp(clName + '(?![-\\\\S])');

              for (; i < len; i += 1) {
                if (!(classRx.test(els[i].className.replace(preRx, '')))) {
                  els[i].className = els[i].className + ' ' + clName;
                }
              }
            }
          }
        }
        return els;
      },

      //----------------------------- Remove Class -------------------------------

      removeClass = function (clName) {

        var i = 0, len, classRX, els = this;

        if(len = els.length) {

          if (clName = (typeof clName === 'string' && trim(clName))) {

            // Use native classList method if available. Not supported in IE8-9.
            if(els[0].classList)

              for (; i < len; i += 1) els[i].classList.remove(clName);

            else {
              // ([\\\\S-])? and the replace function(a,b){..} provide a negative lookbehind
              classRx = new RegExp('([\\\\S-])?'+ clName +'(?: |(?![-\\\\S]))','g');

              for (; i < len; i += 1) {
                els[i].className = trim(els[i].className.replace(classRx, function (a,b){return b ? a : '';}));
              };
            }
          }
        }
        return els; // returns an array of the elements
      };

      // ----------------------- Dom Factory Constructor --------------------------------

      var Dom = function Dom(selector, root) {

        if ( this instanceof Dom ) {

          // Simple Selector [1] #Id, [2] Tag, [3] .class
          var basicSelect = /^#([\\w-]+)$|^([\\w-]+)$|^\\.([\\w-]+)$/,
              mtch,
              els = this;
          // If selector is an HTMl Collection add to Dom Array Like Object
          if (isNodeList(selector)) return push.apply(els, selector);

          if (selector = (typeof selector === 'string' && trim(selector))){

            mch = (selector.match(basicSelect) || '');

            if (mch[1]) {
              els['0'] = getById(mch[1]); els.length = 1;
            }
            else if (mch[2]) {
              push.apply(els, getByTag(mch[2], root));
            }
            else if (mch[3]) {
              push.apply(els, getByClass(mch[3], root));
            }
            // If selector string doesn't match basicSelect use querySelectorAll
            else {
              push.apply(els, queryAll(selector, root));
            }
          }
        return els; // return Dom Array like object
        }
        else return new Dom(selector, root);
      };

      Dom.prototype =  {
        constructor : 'Dom',
        addClass : addClass,
        removeClass : removeClass,
        get : function(){ return slice.call(this); }
      };

      // Add methods to Dom constructor.
      Dom.getById = getById;
      Dom.getByTag = getByTag;
      Dom.getByClass = getByClass;
      Dom.queryAll = queryAll;

  win.Dom = Dom;

}(window, window.document));
//-------------------------------- End of Dom Select -------------------------------------------

// Examples of usage.
// Dom( selector, root ).method

// Using querySelectorAll, additional root selector and
// get() to return an array of html node items.
Dom('li:nth-child(even)', Dom.getById('list1')).get().
  forEach(
    function(i){
      i.style.backgroundColor = 'darkkhaki';
    }
  );

// Select using querySelectorAll and Add a class
Dom('#list2 li:nth-child(odd)').addClass('odd');

// Using a standard HTML Collection as selector
var liItems = Dom.getByTag('li');
Dom(liItems).addClass('whiteText');
</script>
</body>
</html>

Cheers

RLM

Well, a class name will always be preceded by a space or at the beginning of the string, correct? And it will always be followed by a space or at the end of the string. So…

classRx = new RegExp(‘(?:^| )’ + clName + ‘(?:$| )’);

Thanks for the help Jeff.

I’ve over-engineered my reg-ex for sure. Your reg-ex will work fine for testing whether a classname contains a certain class.

Where it can possibly fall down though is if you want to use it for removing class names and you have duplicates like so ‘name name something’. I know it’s an edge case scenario, however the reg-ex will grab the space after the first name on the first match and then fail to match the second name.

I think this and boundaries failing on hyphens in my intitial tests is why I ended up trying other routes.

A few examples.

Using ‘(?:^| )’ + clName + ‘(?:$| )’

// --- (?:^| )' + clName + '(?:$| ) ---
var clName = 'even',
    classRx = new RegExp('(?:^| )' + clName + '(?:$| )','g'),
    aClass = 'even even oddeven even odd-even whiteText even-class'; // OTT I know

// Unfortunately classRx gobbles up the space after the first 'even '<-
// therefore it doesn't make a match with the second one.
// Instead it moves on until it matches the 3rd.
aClass = aClass.replace(classRx, ' ').trim();
                      // even even oddeven even odd-even whiteText even-class
console.log(aClass);  // Output: even oddeven odd-even whiteText even-class

The Jquery like route, which does have the advantage of being consistent across the methods.

// --- Jquery Approach ---
var clName = 'even',
    aClass = 'even even oddeven even odd-even whiteText even-class'; //

aClass = ' ' + aClass + ' '; // pad myClass

while (aClass.indexOf(' ' + clName + ' ') >= 0 ){
  aClass = aClass.replace(' ' + clName + ' ', ' ');
}
                            // even even oddeven even odd-even whiteText even-class
console.log(aClass.trim()); // Output: oddeven odd-even whiteText even-class

And my blagged negative look behind. Unfortunately this can’t be utitilised in a contains method. Well not that I can see.

// --- using a hack negative lookbehind ---
// Unfortunately cannot use this for a contains method.
var clName = 'even',
    classRx = new RegExp('([\\\\S-])?'+ clName +'(?: |(?![-\\\\S]))','g'),
    aClass = 'even even oddeven even odd-even whiteText even-class';

aClass = aClass.replace(classRx, function (a,b){return b ? a : '';}).trim();
                      // even even oddeven even odd-even whiteText even-class
console.log(aClass);  // Output: oddeven odd-even whiteText even-class

As I say ‘(?:^| )’ + clName + ‘(?:$| )’ may well be fine for a contains method. Maybe being a bit OCD on my part.

Cheers

RLM

Absolutely. I suppose I was under the impression that you were trying to find a regex to test if the class already exists, so what I posted didn’t try to do more than that. When it comes to removing classes, I agree the jQuery approach is clever and works well.

If I may suggest, a standard set of class-handling functions exist, that allows you easily do hasClass, addClass, and removeClass on all sorts of older web browsers. Feel free to leverage any of the code from there for backwards compatibility.

Absolutely. I suppose I was under the impression that you were trying to find a regex to test if the class already exists, so what I posted didn’t try to do more than that. When it comes to removing classes, I agree the jQuery approach is clever and works well.

Yes I was. No worries it was me going off on a tangent. Just wanted to try it out:)

Paul, thanks for the links. Essentially it’s using the same regex whereas instead of a space ’ ’ there’s a \s. The \s tests for
and \r as well, so maybe ’ ’ is better.

I do like the use of utilising a hasclass or contains method though.

Just experimenting. I think this isn’t a bad way to go.

if(!!(' ' + elem.className + ' ').indexOf(' ' + clName + ' ')){
  elem.className = elem.className + ' ' + clName;
}

Will do the same. Not sure if better or worse.

if(!~(' ' + elem.className + ' ').indexOf(' ' + clName + ' '))

Albeit there are obvious issues I thought this was interestingly done.
https://gist.github.com/devongovett/1381839

Food for thought
https://www.inkling.com/read/javascript-definitive-guide-david-flanagan-6th/chapter-16/classlist-treat-classname-as-a

So in the end from

preRx = new RegExp('[\\\\S-]' + clName,'g');
classRx = new RegExp(clName + '(?![-\\\\S])');
 
for (; i < len; i += 1) {
  if (!(classRx.test(els[i].className.replace(preRx, '')))) {
    els[i].className = els[i].className + ' ' + clName;
  }
}

to this

for (; i < len; i += 1) {
              
  if(!!(' ' + els[i].className + ' ').indexOf(' ' + clName + ' ')){
    els[i].className = els[i].className + ' ' + clName;
  }
}

A step in the right direction I think.

Cheers again for the help.

Updated code so far…

// ------------------------------------ Dom Select -----------------------------------
(function(win, doc){

  var docElem = doc.documentElement,
      
      slice = [].slice,
      push = [].push,
      toString = {}.toString,
      hasOwn = {}.hasOwnProperty,
      
      // native trim not supported in older versions of IE
      trim = function(strg) { return strg.replace(/^\\s+|\\s+$/g, '') },
      
      objType = function(obj){ return / ([^\\]]+)/.exec({}.toString.call(obj))[1]; },
      
      isNodeList = function(obj) { 
        return /HTMLCollection|HTMLUListElement/.test(objType(obj));
      },
      
      //------------------- Shortcuts to default selectors. ------------------

      getById = function getById(id) { return doc.getElementById(id); },
      
      getByTag = function getByTag(name, root) { return (root||doc).getElementsByTagName(name); },
      
      // Needs polyfill for IE8 and pseudo selectors
      queryAll = function (name, root) { return (root||doc).querySelectorAll(name); },
      
      // Fallback for IE 8 use querySelectorAll instead.
      getByClass = (function(doc, docElem){
                     return (docElem.getElementsByClassName)
                       ? function getByClass(name, root) { return (root||doc).getElementsByClassName(name); }
                       : queryAll; 
                   })(doc, docElem),
      
      //--------------------------- Class Methods -----------------------------
      
      //----------------------------- Add Class -------------------------------
      
      addClass = function (clName) { 

        var i = 0, len, classRx, preRx, els = this;
      
        if(len = els.length) {
        
          if (clName = (typeof clName === 'string' && trim(clName))) {
            // Use native classList method if available. Not supported in IE8-9.
            if(els[0].classList) {

              for (; i < len; i += 1) els[i].classList.add(clName);
        
            } else {
              for (; i < len; i += 1) {
              
                if(!!(' ' + els[i].className + ' ').indexOf(' ' + clName + ' ')){
                  els[i].className = els[i].className + ' ' + clName;
                }
              }
            }
          }
        }
        return els;
      },
    
      //----------------------------- Remove Class -------------------------------
      
      removeClass = function (clName) {

        var i = 0, len, classRX, els = this;
      
        if(len = els.length) {

          if (clName = (typeof clName === 'string' && trim(clName))) {
          
            // Use native classList method if available. Not supported in IE8-9.
            if(els[0].classList)

              for (; i < len; i += 1) els[i].classList.remove(clName);
        
            else {
              // ([\\\\S-])? and the replace function(a,b){..} provide a negative lookbehind
              classRx = new RegExp('([\\\\S-])?'+ clName +'(?: |(?![-\\\\S]))','g');

              for (; i < len; i += 1) {
                els[i].className = trim(els[i].className.replace(classRx, function (a,b){return b ? a : '';}));
              };
            }
          }
        }
        return els; // returns an array of the elements
      };
      
      // ----------------------- Dom Factory Constructor --------------------------------
      
      var Dom = function Dom(selector, root) {

        if (!(this instanceof Dom)) return new Dom(selector, root);

        // Simple Selector [1] #Id, [2] Tag, [3] .class
        var basicSelect = /^#([\\w-]+)$|^([\\w-]+)$|^\\.([\\w-]+)$/,
            mtch,
            els = this;
        // If selector is an HTMl Collection add to Dom Array Like Object  
        if (isNodeList(selector)) return push.apply(els, selector);
          
        if (selector = (typeof selector === 'string' && trim(selector))){

          mch = (selector.match(basicSelect) || '');
            
          if (mch[1]) {
            els['0'] = getById(mch[1]); els.length = 1;
          } 
          else if (mch[2]) {
            push.apply(els, getByTag(mch[2], root));
          }
          else if (mch[3]) {
            push.apply(els, getByClass(mch[3], root));
          }
          // If selector string doesn't match basicSelect use querySelectorAll
          else {
            push.apply(els, queryAll(selector, root));
          }
        }
        return els; // return Dom Array like object
      },
  
      domProto = Dom.prototype =  {
        constructor : 'Dom',
        
        addClass : addClass,
        
        removeClass : removeClass,
        
        get : function(){ return slice.call(this); },
        
        extend: function (from) {
                  var to = arguments[1] || this,
                  prop;
                  
                  for (prop in from) {
                    if (hasOwn.call(from, prop)) {
                      to[prop] = from[prop];
                    }
                  }
                }
      };
      
      // Add methods to Dom constructor.
      domProto.extend ({
        getById : getById,
        getByTag : getByTag,
        getByClass: getByClass,
        queryAll : queryAll
      }, Dom);
       
  win.Dom = Dom;
    
}(window, window.document));

While ’ ’ may be preferred when authoring the HTML, it is legal to use line breaks in your HTML attributes, so for greatest compatibility \s really needs to be done. If you’re designing this though for your own personal use then of course you’re welcome to limit things to any personal preference that you have.

As a small project, I’ve run the code through jslint cleaning things up.

After taking care of many minor issues, the only significant issue was the [^\]] regex, which is better done by saying what is allowed. Replacing that with [\w] seems to work well.

Here’s the above code after the cleanup


(function (win, doc) {
    "use strict";
    var docElem = doc.documentElement,

        slice = [].slice,
        push = [].push,

        // native trim not supported in older versions of IE
        trim = function (strg) {
            return strg.replace(/^\\s+|\\s+$/g, '');
        },

        objType = function (obj) {
            return (/ ([\\w]+)/).exec({}.toString.call(obj))[1];
        },

        isNodeList = function (obj) {
            return (/HTMLCollection|HTMLUListElement/).test(objType(obj));
        },

        //------------------- Shortcuts to default selectors. ------------------

        getById = function getById(id) { return doc.getElementById(id); },

        getByTag = function getByTag(name, root) {
            return (root || doc).getElementsByTagName(name);
        },

        // Needs polyfill for IE8 and pseudo selectors
        queryAll = function (name, root) {
            return (root || doc).querySelectorAll(name);
        },

        // Fallback for IE 8 use querySelectorAll instead.
        getByClass = (function (doc, docElem) {
            return (docElem.getElementsByClassName)
                ? function getByClass(name, root) {
                    return (root || doc).getElementsByClassName(name);
                }
                 : queryAll;
        }(doc, docElem)),

        //--------------------------- Class Methods -----------------------------

        //----------------------------- Add Class -------------------------------

        addClass = function (clName) {

            var i, classIndex, els = this, len = els.length;

            if (len) {

                clName = (typeof clName === 'string' && trim(clName));
                if (clName) {
                    // Use native classList method if available. Not supported in IE8-9.
                    if (els[0].classList) {

                        for (i = 0; i < len; i += 1) {
                            els[i].classList.add(clName);
                        }

                    } else {
                        for (i = 0; i < len; i += 1) {

                            classIndex = (' ' + els[i].className + ' ').indexOf(' ' + clName + ' ');
                            if (!!classIndex) {
                                els[i].className = els[i].className + ' ' + clName;
                            }
                        }
                    }
                }
            }
            return els;
        },

        //----------------------------- Remove Class -------------------------------

        removeClass = function (clName) {

            var i = 0, classRx, els = this, len = els.length, negativeLookbehind;

            if (len) {

                clName = (typeof clName === 'string' && trim(clName));
                if (clName) {

                    // Use native classList method if available. Not supported in IE8-9.
                    if (els[0].classList) {

                        for (i = 0; i < len; i += 1) {
                            els[i].classList.remove(clName);
                        }

                    } else {
                        // ([\\\\S-])? and the replace function (a,b) {..} provide a negative lookbehind
                        classRx = new RegExp('([\\\\S-])?' + clName + '(?: |(?![-\\\\S]))', 'g');
                        negativeLookbehind = function (a, b) {
                            return b ? a : '';
                        };

                        for (i = 0; i < len; i += 1) {
                            els[i].className = trim(els[i].className.replace(classRx, negativeLookbehind));
                        }
                    }
                }
            }
            return els; // returns an array of the elements
        },

        // ----------------------- Dom Factory Constructor --------------------------------

        Dom = function Dom(selector, root) {

            if (!(this instanceof Dom)) {
                return new Dom(selector, root);
            }

            // Simple Selector [1] #Id, [2] Tag, [3] .class
            var basicSelect = /^#([\\w\\-]+)$|^([\\w\\-]+)$|^\\.([\\w\\-]+)$/,
                els = this,
                mch;
            // If selector is an HTMl Collection add to Dom Array Like Object
            if (isNodeList(selector)) {
                return push.apply(els, selector);
            }

            selector = (typeof selector === 'string' && trim(selector));
            if (selector) {

                mch = (selector.match(basicSelect) || '');

                if (mch[1]) {
                    els['0'] = getById(mch[1]);
                    els.length = 1;
                } else if (mch[2]) {
                    push.apply(els, getByTag(mch[2], root));
                } else if (mch[3]) {
                    push.apply(els, getByClass(mch[3], root));
                } else {
                    // If selector string doesn't match basicSelect use querySelectorAll
                    push.apply(els, queryAll(selector, root));
                }
            }
            return els; // return Dom Array like object
        },
        domProto;

    Dom.prototype =  {
        constructor : 'Dom',

        addClass : addClass,

        removeClass : removeClass,

        get : function () { return slice.call(this); },

        extend: function (from, to) {
            var prop;
            to = to || this;

            for (prop in from) {
                if (from.hasOwnProperty(prop)) {
                    to[prop] = from[prop];
                }
            }
        }
    };
    domProto = Dom.prototype;

    // Add methods to Dom constructor.
    domProto.extend({
        getById : getById,
        getByTag : getByTag,
        getByClass: getByClass,
        queryAll : queryAll
    }, Dom);

    win.Dom = Dom;

}(window, window.document));

You’re a star. Nice one man:tup:

What helps a lot is that I have added jslint to my code editor, which makes it really easy to see immediately what may be the matter, with details in the status bar about what to fix.

Which editor is that Paul?

I use Sublime Text that uses a [url=“https://sublime.wbond.net/installation”]package control to install additional packages. In this case, it’s the [URL=“https://sublime.wbond.net/packages/SublimeLinter”]SublimeLinter package followed up with the JSLint linter itself.

Sublime Text is a paid product though and well worth it in my opinion. Given that though there is a separate open-source version that has most of the same features and key commands, called Atom where you can [url=“http://joshbranchaud.com/blog/2014/02/27/Installing-Packages-In-Atom-Editor.html”]install packages and use its [url=“https://github.com/tcarlsen/atom-jslint”]JSLint package instead.

Sublime Text has Python under the hood, whereas GitHub’s Atom use HTML+CSS so is a bit slower. For a free and extensible editor though they’re doing well to keep up with Sublime Text.

Oh hey - and an article has just some out now about Sublime Text packages for front-end web development too.

Thanks Paul, much appreciated. I’m checking it out now.

Well just been through my original code with Sublimelint JsLint.

Need a bit more time to evaluate Sublime. Notepad++ has been pretty good up to now. I guess the live JSlint is nifty though.

Albeit JSLint can spoil your fun it is a worthwhile exercise. Picked up on my accidental global. ClassRx and ClassRX and also some unused variable like {}.toString, which I have now fixed.

Didn’t realise that there were performance penalties with using the arguments object in the way I did, so will have to give it better consideration in future. JSlint didn’t like my hasown.call within ‘for in’ did it?

Still don’t really get the beef with using negated-ed regex’s [^xyz]. Security is mentioned but I’m not sure how that is. As mentioned before this technique is taught in Mastering Regular Expression. That said in future can look at the non greedy *? option as a viable alternative.

In addition and we have discussed this before [-\w]+ should not be a problem in fact from what I have read it is recommended. Again it is taught in the O’Reilly books. Yes it represents a range but not if it’s placed where it is. Well that’s my understanding anyway. Will have to dig out the text on it.

The string replace and function argument within a loop caught me out. I guessing that doesn’t apply to the likes of array.forEach( function(){ }) then? or does it? Easy fix anyway.

There you go.

Cheers

RLM

One of my favourite Sublime Text things is to use shift+f3 to find all occurrences of the word the selection bar is on, which lets you do a multiple-edit of all at the same time.
Likewise, you can use ctrl+d (multiple times) to find and add the next match to the selection, and use multiple cursors on those as well.
The sideshow on their front page gives you a good intro to some of the neat things it can do.

No it didn’t like it - which is to the benefit of the person reading the code. They don’t know if your own hasown function handles things properly without chasing it up to check out separately. If you do want to use a separate function, then make sure that the for loop is in that separate function too.

For example:


function forEachProp(obj, callback) {
    var prop;
    for (prop in obj) {
        if (obj.hasOwnProperty(prop) {
            callback.call(obj, prop);
        }
    }
}
...
forEachProp(from, function (prop) {
    ...
};

It’s a matter of whitelisting versus blacklisting. With [^\]] you saying that you’re going to accept absolutely everything except for a closing square bracket, which includes unicode and all manner of other weird stuff that you do not know you are agreeing to.

A much more beneficial technique is whitelisting instead, such as with [\w] where you declare only that which you are willing to accept, which helps to remove any doubt about what’s going to happen there.

In the specific place that you used it there is little trouble, but using negations are a bad habit to get in to.

You can tell jslint to ignore negations in a certain area, such as with


/*jslint regex:false */
objType = function (obj) {
        return (/ ([^\\]]+)/).exec(...)[1];
    },
/*jslint regex:true */

But that then having such an exception tends to raise even more questions. Doing things properly by positively declaring what you want to capture, results in much less headaches.


objType = function (obj) {
        return (/ ([\\w]+)/).exec(...)[1];
    },

Yes, it’s only on standard for loops where defining a function within the loop is not desired, for performance benefits and an improvement of clarity to the reader of the code.

You may notice that most of these changes don’t affect how the program is run. The main benefactor of these improvements to the code is the future you, and anyone else who works with the code too. The principle of least astonishment is a very nice principle indeed.

One of my favourite Sublime Text things is to use shift+f3 to find all occurrences of the word the selection bar is on, which lets you do a multiple-edit of all at the same time.
Likewise, you can use ctrl+d (multiple times) to find and add the next match to the selection, and use multiple cursors on those as well.
The sideshow on their front page gives you a good intro to some of the neat things it can do.

shift-f3 isn’t working for me, but it appears to do it anyway. If I select a word it then highlights the word elsewhere with an outline. It is very useful. As you say well worth checking out the extra functionality in Sublime… Just tested ctrl-d, very handy.

Regards whitelisting and blacklisting regex’s. I agree to an extent. However it seems to me people are more likely to make an unintentional mistake whitelisting and doing something like this /.*\)/ to match up to a bracket. I agree you need to be aware of what you are actually capturing. Deja-vu here. I think we’ve talked about this before.

Just shot myself in the foot. I just checked over an example from my parser and as a result have found a flaw in my blacklisting approach. Matching comments /\/\[^\\/]\\//gm. This fails on 'Some code here …/* my comments ///here/// /'. However this works a treat /\/\.?\\//gm. You win.:slight_smile:

No it didn’t like it - which is to the benefit of the person reading the code. They don’t know if your own hasown function handles things properly without chasing it up to check out separately. If you do want to use a separate function, then make sure that the for loop is in that separate function too.

Need to give this a bit of a research. With hindsight my extend/mixin function is so simple I didn’t really need to complicate it. Hasown is no major saving, and at this point there is no need for working with arguments.

You may notice that most of these changes don’t affect how the program is run. The main benefactor of these improvements to the code is the future you, and anyone else who works with the code too. The principle of least astonishment is a very nice principle indeed.

Seems a point well worth making especially if you are in a work environment.

I am still a bit torn regarding Jslint though. It’s been in-valuable for picking up on flaws I’ll give it that, but being solely dictated to by Douglas doesn’t sit well with me. Whilst I do like a certain amount of order and clarity JSlint does take an element of creativity out of the process. I’ve actually switched to JSHint now, which seems to pick up on the important stuff whilst allowing a bit more freedom.

For instance you will see Nicholas Zakas do the following in his books, which I don’t see a major issue with.

var i = 0,
      len = arr.length;

for (; i < len; i +=1) { .... }

Additionally assignments instead of conditions with if, while statements etc.

while ( child = parent ) or while ( match = pattern.exec(string) ) seem just fine to me. I know there’s an alternative, but it’s succinct and to the point. You’ll find examples of this in numerous books including the definitive guide.

The only arguments I have found so far against it is that ‘=’ can be confused with ‘==’ or just that Douglas says it’s wrong. Personally I use the stricter ‘===’ for conditions and ‘=’ for assignment. I think they are pretty distinct. Many operators are similar but with different functions && and & for instance.

My opinion may well change with experience but being able to check for a condition and assign in one fell swoop appeals to me.

And lastly calling my if (!!(’ ’ + els[i].className + ’ ‘).indexOf(’ ’ + clName + ’ ')) weird is just down right insulting. ha ha.

Cheers again Paul. Your feedback and advice is appreciated.

RLM

Whoops, I got my shifts and alts mixed up. Try it with alt+F3, or the menu option Find -> Quick Find All

That’s why using the .* selector is not accepted by jslint either - for it allows far too much to be accepted which becomes poor security yet again. Instead, consider what you would allow up to the closing parenthesis, which could be as broad as [/w/W] and match for that.

The issue there is that you don’t know where i is starting from, without tracing back through the code, which if the for loop is further down in the method could be quite a bit of work.
Another problem is that you can break out of a for loop prematurely. If you do that and you then expect a further for loop to start at 0, you will end up with some difficult to debug problems. You don’t even need to break out of the loop to cause problems either. Just having two for loops in a row will create problems when the initialization is left out, for the second loop will have i start at the end of the first one.

Keep surprises to a minimum and make it clear to the loop and to the people reading it where things to begin.


for (i = 0; i < len; i +=1) {

It helps to protect your future self from problems, and leaves the reader with no misplaced assumptions about what’s going to happen there.

BTW, thumbs up for using +=1 instead ++ :tup:

The problem with that code is that when trying to find and fix a bug, it’s not clear if you intended the assignment to occur there in the first place. Some hard to find bugs have ended up being an assignment instead of a comparison.

It’s good then that jslint lets you allow assignment expressions. Just place the following code at the top of your code and it won’t say anything about them.


/*jslint ass:true */

The same goes for other aspects of jslint that you disagree with too. Later on if you’re bitten too much by problems, you can update the comment and such areas will be easily able to be found an fixed too.