get rid of useless for..in loop in getStyle #2410

Merged
merged 1 commit into from Aug 20, 2012

2 participants

@gonchuki

as discussed in #2409, there was this low hanging fruit that we can harvest right now without going through any major refactor that could break stuff.
Unless I'm missing something in the previous logic, this is totally safe to check in.

@ibolmo ibolmo commented on the diff Aug 20, 2012
Source/Element/Element.Style.js
@@ -114,10 +114,9 @@ Element.implement({
property = (property == 'float' ? floatName : property).camelCase();
var result = this.style[property];
if (!result || property == 'zIndex'){
- result = [];
- for (var style in Element.ShortStyles){
- if (property != style) continue;
- for (var s in Element.ShortStyles[style]) result.push(this.getStyle(s));
+ if (Element.ShortStyles.hasOwnProperty(property)){
@ibolmo
MooTools member
ibolmo added a note Aug 20, 2012

Particular reason why not just:

if (Element.ShortStyles[property]){

?

defensive coding, if the Object prototype ever gains a property named exactly like a CSS style that piece of code won't blow up into pieces. That includes both future standards and nasty users/3rd party scripts adding stuff to the Object prototype.

@ibolmo
MooTools member
ibolmo added a note Aug 20, 2012

That's a good idea then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ibolmo ibolmo merged commit d2a662d into mootools:master Aug 20, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment