3

I've got a function in an object like this:

arrayofElements: function(item) {

    var result = [];

    if (item.isA) {
        result.push(new Div('aClass', labels['A']));
    }

    if (item.isC) {
        result.push(new Div('cClass', labels['C']));
    }

    if (item.isD) {
        result.push(new Div('dClass', labels['D']));
    }

    return result;
},

How can this be refactored? I dislike having to push() each item conditionally.

3
  • What other properties does item have? Commented Feb 1, 2010 at 4:21
  • 2
    Do you dislike the .push() method? or the conditions wrapping each call? Commented Feb 1, 2010 at 4:35
  • Is the labels property accessible from within an item object? Commented Feb 1, 2010 at 10:19

4 Answers 4

4

You could move the is* properties to a sub-object so as to isolate them, and loop over the sub-object's properties

item.is={A:'aClass', C:'cClass'};
...
arrayofElements: function(item) {
    var result = [];
    for (p in item.is) {
        result.push(new Div(item.is[p], labels[p]));
    }
    return result;
},

The values in item.is could be the classes (as shown), or the values in item.is could be objects with a class property, or you could use the property name p as an index in another object to get the classes. Which depends largely on what the items represent and what the element classes are most closely associated with.

Sign up to request clarification or add additional context in comments.

Comments

1
arrayofElements: function(item) {

var result = [],
    items = ['A', 'C', 'D'];
while(var i=0; i < items.length; i++) {
  if(item["is" + items[i]]) {
    result.push(new Div(items[i] + 'Class', labels[items[i]]))
  }
}
return result;
},

Remember that obj.foo is the same as obj['foo'] in Javascript

Comments

0

A variant of Larry's solution:

arrayofElements: function(item) {
  var result = [], letters = ['A','C','D'], ltr, i, len;
  len = letters.length;
  for (i = 0; i < len; i++) {
    ltr = letters[i];
    if (item["is"+ltr]) {
      result.push(new Div(ltr.toLowerCase()+"Class", labels[ltr]));
    }
  }
  return result;
},

Comments

0

This function avoids using push() and is functionally identical to yours:

arrayofElements: function(item) {

    if (item.isA) {
       return [new Div('aClass', labels['A'])];
    }

    if (item.isC) {
       return [new Div('cClass', labels['C'])];
    }

    if (item.isD) {
       return [new Div('dClass', labels['D'])];
    }

    return [];
},

2 Comments

not quite. In his code if item looks like {isA:true,isC:true} it will return [new Div('aClass', labels['A']),new Div('cClass',labels['C'])] Array#push modifies the array it is called on.
Ah, you're right. I assumed that an item could only be true for one of isA, isC, or isD.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.