Regex match for a string in a URL

Question

I feel like there is too much repetitive code going on here. All I am doing is doing a basic regex match for a string in the URL.

If a match is found, I find a li with a class (.index, .grid or .type) and add the active class. This is just for my main nav in an attempt to make it somewhat dynamic.

However, I feel like there is a more efficient way to code this:

$(document).ready(function() {
 var myLocation = window.location.href;
 var index = /index/i;
 var grid = /grid/i;
 var type = /type/i;
 var urlIndex = convertURL.match(index);
 var urlGrid = convertURL.match(grid);
 var urlType = convertURL.match(type);
 
 if (urlIndex) {
 $('.index').addClass('active');
 } else if (urlGrid) {
 $('.grid').addClass('active');
 } else if (urlType) {
 $('.type').addClass('active');
 }
});

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

Answer 1

Much like elclanrs' solution, just slightly more "plain"

$(function () {
 var classes = ['index', 'grid', 'type'],
 url = window.location.href.toLowerCase();
 for(var i = 0, l = classes.length ; i < l ; i++ ) {
 if(url.indexOf(classes[i]) !== -1) {
 $('.' + classes[i]).addClass('active');
 break; // only first matching class is considered
 }
 }
});

• user3259232: so without the break, all the li's would get the class "active" right?
• David Harkness: @user3259232 All the ones with a class matching the URL would, yes. E.g. foo.com/bar/index/how-to-griddle.html would activate the index and grid items, but not the type ones..
• Flambino: @user3259232 Without the break, the code would work like a series of ifs, instead of an if.. else if structure like your code. I.e. without the break a url like example.com/index/grid/type/xyz would cause it to act the same as $(".index, .grid, .type").addClass("active") because all three classes would get matched. But with the break, only the first matching class will be considered, so it's like $(".index").addClass("active")
• David Harkness: @Flambino My comment was an answer to user3259232's comment about removing the break.
• Flambino: @DavidHarkness Makes sense. Sorry - failed to read it in its proper context

Answer 2

I think you meant myLocation in place of convertURL.

You don't need that many variables, a typical refactoring might look like this:

// Short version of $(document).ready(fn)
$(function() {
 var loc = window.location.href;
 if (/index/i.test(loc)) {
 $('.index').addClass('active'); 
 } else if (/grid/i.test(loc)) {
 $('.grid').addClass('active');
 } else if (/type/i.test(loc)) {
 $('.type').addClass('active');
 }
});

But I would go for a less imperative solution, without the use of regex, for example:

$(function() {
 var loc = window.location.href;
 var classes = ['index', 'grid', 'type'];
 var isMatch = function(x){return loc.toLowerCase().indexOf(x)>-1};
 $('.'+ classes.filter(isMatch)[0]).addClass('active');
});

Note that you can go even further an cache the lowercased url, but you may want to use the original variable intact later in the code.

• David Harkness: The first rewrite is missing the else part of the original else if tests. This could activate up to three items. In the second, you may as well convert the URL to lowercase when pulling it from window.location.href instead of repeating the process for every indexOf call.
• elclanrs: @DavidHarkness: yes, I edited and made them if without realizing. And yes, I thought about doing toLowerCase above but OP might want to use the url in its original shape later on.
• David Harkness: Keep in mind also that Array.prototype.filter is defined in ES5. You can use es5-shim for older browsers.

Answer 3

$(document).ready(function () {
 // use single var per function, 
 // good for minimizing and other stuff
 var
 i,
 // new string literal, not String object
 convertURL = '' + window.location,
 // the array of strings keeps only the difference 
 // from the repetitive code
 classes = ['index', 'grid', 'type'],
 // using functions with proper arguments reduces repetitivness
 matches = function (regex) {
 return convertURL.match(new RegExp(regex, 'i'));
 }
 // var
 ;
 // always use += instead of ++ -> makes for clear intention
 for (i = 0; i < classes.length; i += 1) {
 if (matches(classes[i])) {
 // returning out of this function stops iteration
 return $('.' + classes[i]).addClass('active');
 }
 }
});

• David Harkness: It seems a bit wasteful to use regex when a simple indexOf on the lowercased URL would suffice.
• Azder: At this moment I don't see it as a concern because DOM manipulation is so much slower that it will negate all of the micro-optimizations :/