Understanding module design patterns in javascript

Question

I am trying to understand module patterns in Javascript so that i can separate my code into different modules and use them where required.

var messageHandler = (function(){
    var el;
    var display = function(a){
        if(a=='error'){
            el = $('.error');
            el.css('display','block');
        }
        else if (a==='success'){
            el = $('.success');
            el.css('display','block');
        }
        else if (a=='warning'){
            el = $('.warning');
            el.css('display','block');
        }
        else if (a=='danger'){
            el = $('.danger');
            el.css('display','block');
        }
        registerClick(el.find('.close'));
        return this;
    }
    function registerClick(p_el){
        p_el.bind('click',function(){
            hide();
        });
    }
    var hide = function(){
        el.css('display','none');
    }
    return {
        display: display,
        hide: hide
    }
})();
window.messageHandler = messageHandler;

messageHandler.display('warning');

So, I have four different classes in css for different types of messages.The close class is for a small cross button on the top right to close the message. This works fine till i call the function only once.When i do this

messageHandler.display('warning');
 messageHandler.display('success');

Now both the messages close button have been bind to the success close button because el gets overwritten.

How to achieve it keeping the code reusable and concise.

Answer 1

The problem here is that you have a closure variable el that you are overwriting every time display() is called. The hide() function uses whatever is the current value of el at the time it is called, so overwriting el is a problem.

If you want to have "static" functionality like this display() method, you need to avoid shared state.

As @Bergi points out in the comments, you can eliminate the shared el and modify hide() to take an element as input:

var messageHandler = (function(){
    var el;     // delete this
    var display = function(a){
        var el; // add this

function registerClick(el){
    el.bind('click', function(){
        hide(p_el);
    });
}
function hide(el){
    el.css('display','none');
}

You could also modify hide to make use of the current event properties, and then just have:

function registerClick(el){
    el.bind('click', hide);
}

function hide(event){
    $(event.target).css('display','none');
}

Cleaned up version including the auto-hide discussed in the comments:

var messageHandler = (function(){
    var display = function(a){
        var el = $('.' + a);
        el.css('display', 'block');

        var hideAction = function () { el.css('display', 'block'); };
        var token = setTimeout(hideAction, 5000);

        el.find('.close').bind('click', function () {
            hideAction();
            clearTimeout(token);
        });

        return this;
    }

    return {
        display: display
    }
})();