patternjavascriptMinor
addEventListener - v0
Viewed 0 times
addeventlistenerstackoverflowprogramming
Problem
The facade pattern is used to abstract adding events. Should support IE5+ as stands.
Looking for minor improvements. Don't need to support prior to IE5.
I choose not to use a library.
Looking for minor improvements. Don't need to support prior to IE5.
I choose not to use a library.
/*addEL
** dependencies - none
** browser - IE5+
** notes - improved per answer
*/
NS.addEL = (function binding() {
if (window.addEventListener) {
return function addEventListener(element, type, callNow) {
element.addEventListener(type, callNow);
};
}
if (window.attachEvent) {
return function attachEvent(element, type, callNow) {
element.attachEvent('on' + type, callNow);
};
}
}());Solution
-
Why does binding have parameters? The closure isn't passing any.
-
As far as I know, 75% of browsers (including IE9+) actually support
-
For better readability, use common conventions when naming. Element references can be shortened to
-
Although you might say that you are not going to support older/other browsers, that does not mean you should recklessly code.
In your code, if you used
So I added a third case which makes
And so:
Another approach to this is the following code. It's shorter, but performs checks everytime the function is called. If the browser supports neither, then nothing is actually executed:
Why does binding have parameters? The closure isn't passing any.
-
As far as I know, 75% of browsers (including IE9+) actually support
addEventListener. If you check for attachEvent first, you are wasting that 75% assurance. Instead, check for addEventListener first and 75% of the time, your code is evaluating just one condition.-
For better readability, use common conventions when naming. Element references can be shortened to
el, events are usually ev (because e sometimes refer to error objects), and event handlers are usually called handler or callback.-
Although you might say that you are not going to support older/other browsers, that does not mean you should recklessly code.
In your code, if you used
attachEvent was not supported, you use addEventListener. But what if the browser hasn't got addEventListener either? The binding will crash trying to use it since addEventListener a last resort that is, in this case, not supported. So I added a third case which makes
NS.addEL be a function that does nothing if both methods are not available.And so:
NS.addEL = (function binding(){
if(window.addEventListener){
return function addEventListener(el, ev, handler){
el.addEventListener(ev, handler);
}
}
if(window.attachEvent) {
return function attachEvent(el, ev, handler){
el.attachEvent('on' + ev, handler);
}
}
return function(){/*not supported for some reason*/}
}());Another approach to this is the following code. It's shorter, but performs checks everytime the function is called. If the browser supports neither, then nothing is actually executed:
NS.addEL = function addEL(el,ev,handler){
if(window.addEventListener) {
el.addEventListener(ev,handler);
} else if(window.attachEvent){
el.attachEvent('on' + ev,handler);
}
});Code Snippets
NS.addEL = (function binding(){
if(window.addEventListener){
return function addEventListener(el, ev, handler){
el.addEventListener(ev, handler);
}
}
if(window.attachEvent) {
return function attachEvent(el, ev, handler){
el.attachEvent('on' + ev, handler);
}
}
return function(){/*not supported for some reason*/}
}());NS.addEL = function addEL(el,ev,handler){
if(window.addEventListener) {
el.addEventListener(ev,handler);
} else if(window.attachEvent){
el.attachEvent('on' + ev,handler);
}
});Context
StackExchange Code Review Q#23282, answer score: 2
Revisions (0)
No revisions yet.