I'll try to keep it brief. I'm alternating between two event listeners, one that calls "goMetric" and the other that calls "goImperial". The unitSwitch is the link that switches between the two. Is there a more efficient way to remove one event listener and replace it with another? Any other tips on how to write better code are greatly appreciated.

(note: am trying to avoid jQuery for now, in order to better understand JavaScript. I want to know what goes on under the hood.)

var isMetric = false;
unitSwitch.addEventListener('click', goMetric);

function goMetric() {
    isMetric = true;
    // removed other code
    unitSwitch.innerHTML = "Go imperial";
    unitSwitch.removeEventListener('click', goMetric);
    unitSwitch.addEventListener('click', goImperial);
}

function goImperial() {
    isMetric = false;
    // removed other code
    unitSwitch.innerHTML = "Go metric";
    unitSwitch.removeEventListener('click', goImperial);        
    unitSwitch.addEventListener('click', goMetric);
}

Just seems like a lot of code to do something so simple.

I'll try to keep it brief. I'm alternating between two event listeners, one that calls "goMetric" and the other that calls "goImperial". The unitSwitch is the link that switches between the two. Is there a more efficient way to remove one event listener and replace it with another? Any other tips on how to write better code are greatly appreciated.

(note: am trying to avoid jQuery for now, in order to better understand JavaScript. I want to know what goes on under the hood.)

var isMetric = false;
unitSwitch.addEventListener('click', goMetric);

function goMetric() {
    isMetric = true;
    // removed other code
    unitSwitch.innerHTML = "Go imperial";
    unitSwitch.removeEventListener('click', goMetric);
    unitSwitch.addEventListener('click', goImperial);
}

function goImperial() {
    isMetric = false;
    // removed other code
    unitSwitch.innerHTML = "Go metric";
    unitSwitch.removeEventListener('click', goImperial);        
    unitSwitch.addEventListener('click', goMetric);
}

Just seems like a lot of code to do something so simple.

• javascript

Share Improve this question Follow asked Nov 8, 2013 at 6:55 kfrncskfrncs 46344 silver badges1515 bronze badges 3

• 4 Why not just use a single event listener that checks the variable with an if and does the appropriate thing? – Barmar Commented Nov 8, 2013 at 6:59
• use jquery toggle, api.jquery./toggle . it is easy to switch between the two functions – Paritosh Commented Nov 8, 2013 at 7:01
• I don't think that you can make it more efficient without loosing readability. But instead of adding and removing the listeners I would only add one calling it e.g. toggleGo and depending on the state of isMetric do the corresponding action. That way the event handler name already makes clear what you do there. – t.niese Commented Nov 8, 2013 at 7:09

Add a ment  | 

3 Answers 3

Sorted by: Reset to default 4

IMO a better approach would be just using a single listener and merging the two

function toggleMetricImperial() {
    isMetric = !isMetric;
    unitSwitch.innerHTML = isMetric ? "Go Imperial" : "Go Metric";
    ... other code ...
}

but for removing one listener and setting another, yes, you need to remove one listener and setting another :-)

If this is very mon in your code then you could factor out this need in a library like

function switcher(first, second) {
   var which = second;
   return function() {
       which = (which == first ? second : first);
       return which.apply(null, arguments);
   }
}

thus by setting the listener to switcher(goImperial, goMetric) instead you would get what you are looking for. Of course in this case setting the button title would be part of the "... other code ..." section.

switcher is a called an high-order function, that given two functions returns a function that when called will call one or the other alternating them. Note that given Javascript rules for this you may need to wrap the handler in a function that calls the handler body passing the correct this. In Javascript for example calling obj.meth(); is not the same as var m=obj.meth; m();.

I would write this like this:

var isMetric = false;
unitSwitch.addEventListener('click', onClick);

function onClick()
{
    isMetric = !isMetric;
    unitSwitch.innerHTML = (isMetric ? "Go metric" : "Go imperial");
}

There is no need for two distinct event handlers for state dependent action.

Try something simple like this... This only needs one listener, assigned to a static function that deals with the toggling logic. rcpaul had mentioned something similar, but in shorthand form.

var isMetric = false;
unitSwitch.addEventListener('click', toggleUnit);

function goMetric() {
    isMetric = true;
    unitSwitch.innerHTML = "Go imperial";
}

function goImperial() {
    isMetric = false;
    unitSwitch.innerHTML = "Go metric";      
}

function toggleUnit() {
    if (isMetric) {
        goImperial();
    } else {
        goMetric();
    }
}

// Init
goMetric();