0
\$\begingroup\$

I have been attempting to have a method call, with a parameter, inside of a ternary operator to simplify some code.

Code:

 if (window.addEventListener) {
     btnArray[index].addEventListener('click', function() {
        doMassClick( btnArray[index], nameArray[index]);
     });
 } else {
     btnArray[index].attachEvent('onclick', function() {
        doMassClick( btnArray[index], nameArray[index]);
     });
 }

I was able to simplify this down to:

btnArray[index][ window.addEventListener ? 'addEventListener' : 'attachEvent'](window.addEventListener ? 'click' : 'onclick', function() {
    doMassClick( btnArray[index], nameArray[index]);
});

I'd love to know how this can be simplified/cleaned up more..

\$\endgroup\$
  • \$\begingroup\$ I am creating this because it needs to be compatible with IE8... I am using basic JS here, no libraries/dependencies. \$\endgroup\$ – sarcastasaur Apr 6 '18 at 20:11
  • 2
    \$\begingroup\$ I'm struggling to understand why it would need to be compatible such ancient IE version. Why not make it also compatible with IE6 and Netscape ? \$\endgroup\$ – Isac Apr 6 '18 at 20:23
  • \$\begingroup\$ because IE6 is too old, get with the program... \$\endgroup\$ – sarcastasaur Apr 6 '18 at 20:28
  • \$\begingroup\$ @Isac I have to support IE 8 because some of our very large corporate clients use it. Some still use IE 6, but we pushed back on having to support that, thank goodness (the TPMs did--not management). \$\endgroup\$ – Hosch250 Apr 6 '18 at 20:41
  • 2
    \$\begingroup\$ I wouldn't call this a simplification. It's like 10 times more difficult to read then before. \$\endgroup\$ – t3chb0t Apr 6 '18 at 21:05
4
\$\begingroup\$

The common thing in your code is the listener.

function massClickListener() {
    doMassClick( btnArray[index], nameArray[index]);
}

Then it is easier to use the ternary:

btnArray[index][ window.addEventListener ? 'addEventListener' : 'attachEvent'](window.addEventListener ? 'click' : 'onclick', massClickListener);

Personally however, I think this is better:

if (window.addEventListener) {
    btnArray[index].addEventListener('click', massClickListener);
} else {
    btnArray[index].attachEvent('onclick', massClickListener);
}

Using the conditional operator to choose different methods is not very readable.

Another option might be to use a framework that does these kinds of things for you.

\$\endgroup\$
  • \$\begingroup\$ Good suggestion, I'd also like to see something like this wrapped into a function for easy re-usability, though #NamingThings ... ie8FriendlyAddEventListener maybe? \$\endgroup\$ – Phrancis Apr 6 '18 at 23:32
3
\$\begingroup\$

Some points

  • window is the default object. You never need to reference it unless you are using bracket notation, or the current scope has an name that has replaced the window.name. Thus if (window.addEventListener) { is identical to if (addEventListener) {

  • addEventListener and attachEvent are both properties of the EventTarget.prototype and should be used to determine which is being used rather than the global object window

  • You only do the test for which function to use once at startup. When ever you find yourself doing the same thing over and over then it is a sure sugn there is a simpler way.

Simpler solution.

This solution assumes you are not using any libraries that manage listeners (but then you would not need it).

To solve the problem execute the following at the start of your code.

if (!EventTarget.prototype.addEventListener) {         
    EventTarget.prototype.addEventListener = function(eventName, callback) {
        this.attachEvent("on" + eventName, callback);
    }
}

Now you can add a listener using addEventListener for any target in IE < 9

btnArray[index].addEventListener("click", massClickListener);

You may wish to do the same with detachEvent .

if (!EventTarget.prototype.addEventListener) {         
    EventTarget.prototype.addEventListener = function(eventName, callback) {
        this.attachEvent("on" + eventName, callback);
    }
    // assumes removeEventListener does not exist if addEventListener
    EventTarget.prototype.removeEventListener = function(eventName, callback) {
        this.detachEvent("on" + eventName, callback);
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ that's a great idea. I'll do that from now on with these browser compat issues I must deal with. thanks for the explanation. \$\endgroup\$ – sarcastasaur Apr 7 '18 at 15:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.