Simplifying Javascript when having similar functions

Question

I've finally pieced together some code that works for my question here. However, it appears pretty long as I've created separate functions for when a page with a certain hash is refreshed and when that same page is accessed via clicking the tabs.

$(document).ready(function () {
    $(function () {
        var loc = window.location.href; // For when Hazel is refreshed
        if (/Hazel/.test(loc)) {
            $("#tab1,#tab2,#tab3,#tab4").removeClass("r p c").addClass("h");
            $("#tab2").removeClass("tail");
            $("#tab3, #tab4").addClass("tail");
        }
    });
    $(function () {
        var loc = window.location.href; // For when Red is refreshed
        if (/Red/.test(loc)) {
            $("#tab1,#tab2,#tab3,#tab4").removeClass("h p c").addClass("r");
            $("#tab3, #tab2").removeClass("tail");
            $("#tab4").addClass("tail");
        }
    });
    $(function () {
        var loc = window.location.href; // For when Pink is refreshed
        if (/Pink/.test(loc)) {
            $("#tab1,#tab2,#tab3,#tab4").removeClass("h r c").addClass("p");
            $("#tab3, #tab4").removeClass("tail");
            $("#tab2").addClass("tail");
        }
    });
});
$(function () {
    var loc = window.location.href; // For when Cyan is refreshed
    if (/Cyan/.test(loc)) {
        $("#tab1,#tab2,#tab3,#tab4").removeClass("h r p").addClass("c");
        $("#tab4").removeClass("tail");
        $("#tab3, #tab2").addClass("tail");
    }
});
$("#tab1").click(function () {
    $(window).bind("hashchange", function () {
        var loc = window.location.href; // For when Hazel tab is clicked
        if (/Hazel/.test(loc)) {
            $("#tab1,#tab2,#tab3,#tab4").removeClass("r p c").addClass("h");
            $("#tab2").removeClass("tail");
            $("#tab3, #tab4").addClass("tail");
        }
    });
});
$("#tab2").click(function () {
    $(window).bind("hashchange", function () {
        var loc = window.location.href; // For when Red tab is clicked
        if (/Red/.test(loc)) {
            $("#tab1,#tab2,#tab3,#tab4").removeClass("h p c").addClass("r");
            $("#tab3, #tab2").removeClass("tail");
            $("#tab4").addClass("tail");
        }
    });
});
$("#tab3").click(function () {
    $(window).bind("hashchange", function () {
        var loc = window.location.href; // For when Pink tab is clicked
        if (/Pink/.test(loc)) {
            $("#tab1,#tab2,#tab3,#tab4").removeClass("h r c").addClass("p");
            $("#tab3, #tab4").removeClass("tail");
            $("#tab2").addClass("tail");
        }
    });
});
$("#tab4").click(function () {
    $(window).bind("hashchange", function () {
        var loc = window.location.href; // For when Cyan tab is clicked
        if (/Cyan/.test(loc)) {
            $("#tab1,#tab2,#tab3,#tab4").removeClass("h r p").addClass("c");
            $("#tab4").removeClass("tail");
            $("#tab3, #tab2").addClass("tail");
        }
    });
});
});

Is it possible to simplify it? I've tried but in my attempts so far, the code just breaks.

Why have you wrapped the first three addOnDOMready-statements in another $(document).ready? — Bergi, Jun 14 '12 at 09:30
this rather belongs on http://codereview.stackexchange.com/ than SO. — Christoph, Jun 14 '12 at 09:30
Is it possible that none of the four colors is refreshed? Or will there always be one the the four regexps within the URL? I'm thinking about an if-elseif-elseif-else. — sp00m, Jun 14 '12 at 09:50
@Bergi: I'll see if I can replicate them as I've already reverted my save file to the above code as it works for me so far; but as per Christoph's suggestion, I think I'll have to ask this question to Code Review instead. — Clarice, Jun 14 '12 at 10:32
@sp00m: This is actually a tabbed menu, so one of the regexps must always be within the URL. — Clarice, Jun 14 '12 at 10:34
@Christoph: Thank you very much for directing me to that site. I suppose it's the better forum for my question as I answered 'yes' to all the on-topic questions on that site :). — Clarice, Jun 14 '12 at 10:35

Sujay · Accepted Answer · 2012-06-20T06:34:25.480

1

I'm assuming that you're using some kind of a JS routing library.

Try this:

$(document).ready(function () {
    var tabInfo = {
            Hazel: {nonTail:'#tab2',tail:'#tab3, #tab4'},
            Red: {nonTail:'#tab3,#tab2',tail:'#tab4'},
            Pink: {nonTail:'#tab3,#tab4',tail:'#tab2'},
            Cyan: {nonTail:'#tab4',tail:'#tab2,#tab3'}
    };
    function makeChanges() {
       var loc = window.location.href; 
       for(var tab in tabInfo){
           if(loc.indexOf(tab) !== -1){
             $("#tab1,#tab2,#tab3,#tab4").removeClass("h r p c").addClass(tab.toLowerCase().charAt(0));
             $(tabInfo[tab].nonTail).removeClass("tail");
             $(tabInfo[tab].tail).addClass("tail");
             break;
           }
        }
    }
    makeChanges();
    $(window).bind("hashchange", function () {
         makeChanges();
    });
});

edited Jun 20 '12 at 06:34

answered Jun 14 '12 at 12:28

Sujay

2,198
23
32

I'm actually really impressed by how succinct your approach is and it partly works. Addition and removal of the h,r,p & c classes is perfect, but addition and removal of the tail class does not work so far. I've gone through the code though and nothing seems amiss. – Clarice Jun 16 '12 at 06:19
@Clarice Is there some kind of an error reported in the console that you can share? Maybe you could add some console.log debug statements inside the if block before break to see if the properties are properly set. Do let me know, so that I can help you further. N sorry for the delay in my response. – Sujay Jun 18 '12 at 20:39
No problem with the delay so don't worry. I'm told that 'tabInfo.tab is undefined' when I click on the tabs. – Clarice Jun 19 '12 at 14:14
@Clarice Can you create a jsfiddle of your code? It'll be easier to debug. I feel that you've either not completed the `tabInfo` definition by adding the configuration for colors other than `'Hazel'` & `'Red'`. Also does this happen for all the tabs? Can you also provide the browser & version that you're testing this on? – Sujay Jun 19 '12 at 17:04
Here's the jsfiddle: [link](http://jsfiddle.net/8WaTQ/1/). I simplified the css for it, but as you can see, basically nothing happens with the tail class (which should be visible or not on hover) no matter which tab is clicked. I'm testing this on Chrome 19, Firefox 6, and IE 9. – Clarice Jun 20 '12 at 03:45
@Clarice I've fixed it [here](http://jsfiddle.net/8WaTQ/3/) & Also edited the answer here. Hope this works fine for you. – Sujay Jun 20 '12 at 06:36
It works perfectly now. As you may have noticed, I'm a newbie with all this, so thank you so much not just for the answer but for your patience as well. – Clarice Jun 20 '12 at 11:33
@Clarice :) Next time maybe you can provide a fiddle in the beginning itself, that way there is a better probability that you'll get faster results. – Sujay Jun 25 '12 at 06:22
Will do. TBH, I wasn't really aware of jsfiddle before, haha. :D Thanks again! – Clarice Jun 26 '12 at 08:16

score 0 · Answer 2 · answered Jun 14 '12 at 12:01

This is untested psuedo code because I wasn't sure how to test the location part. I tried to take an object oriented approach to your problem, moving all of the tab configurations into objects.

var tabs = [{
    "id": "#tab1",
    "color": "hazel",
    "removeTabIds": "#tab1,#tab2,#tab3,#tab4",
    "removeClasses": "r p c",
    "addClasses": "h",
    "removeTailIds": "#tab2",
    "addTailIds": "#tab3 #tab4"
}, { //Other tab configs go here ...
}];

$(document).ready(function(){
    var loc = window.location.href;
    var tab = getTab(loc);
    changeClasses(tab);
});

function getTab(loc){
    for (var i = 0; i < tabs.length; i++) {
        if (/tabs[i].color/.test(loc)) {
            return tab[i];
        }
    }
};

function changeClasses(tab){
    $(tab.removeTabIds).removeClass(tab.removeClasses).addClass(tab.addClasses);
    $(tab.removeTailIds).removeClass("tail");
    $(tab.addTailIds).addClass("tail");
};

function bindTab(tab){
    $(tab.id).click(function(){
        $(window).bind("hashchange", function(){
            var loc = window.location.href;
            var tab = getTab(loc);
            changeClasses(tab);
        });
    });
}

Simplifying Javascript when having similar functions

2 Answers2