tags:

views:

127

answers:

5

Say, I want to see if a DOM element is a block. I can write it in three ways, depending on my mood:

// first way
if (el.currentStyle.display == "block" || el.currentStyle.display == "inline-block" || el.currentStyle.display == "table-cell")       

// second way
var blocks = {"block": 1, "inline-block": 1, "table-cell": 1};
if (el.currentStyle.display in blocks)// 

// third way
if (el.currentStyle.display.match(/block|inline-block|table-cell/))

I have mixed feeling about all of them. First is too verbose once I have more than one option. Second contains those arbitrary values in the object (where I put 1s this time). Third looks like overkill. (What exactly is bad about overkilling?)

Do you know another, better way? If no, any cons I am missing about these three ways?

Javascript only, please.

A: 

Can't you use the 2nd way but check if it's undefined and then skip the ": 1" part. I haven't tested though.

Ólafur Waage
I tried. Without : 1, you get syntax error.
buti-oxa
+2  A: 

I like the third way; I don't think it looks like overkill at all. If you need an even shorter way then this works too:

el.currentStyle.display.match(/(e-)?(block|cell)/)

But that's not very readable...

It might be worth abstracting it all away by extending the String prototype:

String.prototype.matches = function(what) {
    return (',' + what + ',').indexOf(',' + this + ',') > -1;
};

// Using it:
el.currentStyle.display.matches('block,inline-block,table-cell');
J-P
Extending the String prototype may be even bigger overkill, but I love the resulting usage. Very clever.
buti-oxa
A: 

It looks like you need an inArray function, here is one from the top search result:

Array.prototype.inArray = function (value) {
  var i;
  for (i=0; i < this.length; i++) {
    if (this[i] === value) {
      return true;
    }
  }
  return false;
};

Then the forth way would look like this:

if (['block','inline-block','table-cell'].inArray(el.currentStyle.display))

Or in a more readable manner:

var isBlock = ['block','inline-block','table-cell'].inArray(el.currentStyle.display);
pjesi
Or var block = {'block':1, 'inline-block':1, 'table-cell':1}; if( foo in block ) ...
Anonymous
A: 

My prefered solution for this is:

'block||inline-block||table-cell'.indexOf( el.currentStyle.display ) >= 0

I think that this will use native code of the string and be way more efficient than the array & iteration method.

Laurent Villeneuve
I like JimmyP's take on the same idea.
buti-oxa
+1  A: 

If we're primarily aiming for readability, and if this is happening more than once -- perhaps even if it is just once -- I'd move the test to a function. Then define that function whichever way you like -- probably option 1, for max simplicity there.

Overkill? Possibly. But a gift to the programmer who wants to scan and understand the code 6 months from now. Probably you :-)

function isBlock(el) {
  return (el.currentStyle.display == "block" || 
          el.currentStyle.display == "inline-block" || 
          el.currentStyle.display == "table-cell");
}


// ...

if (isBlock(el)) {
  // do something
}
Paul Roub