Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

What would be the best way to format this code? It looks a bit messy to me in regards to indentation etc.

 if (typeof _srwc != 'undefined') {
    for (var i=0; i < _srwc.length; i++){
         var currentArg = _srwc[i];;        
          if (typeof window[currentArg[0]] == 'function') {
             window[currentArg[0]](currentArg[1], currentArg[2], currentArg[3]);
             } else {
             console.log('The Setter method: "' + currentArg[0] + '" is undefined');
             }
    }
 }   
share|improve this question
add comment

2 Answers 2

How to format code is not really how CodeReview works ;)

  • You are indenting with 2 and with 4 spaces, pick one, I advocate 2
  • It is more common to compare _srwc to undefined itself
  • _srwc is an unfortunate name, it conveys no meaning
  • window[currentArg[0]](currentArg[1], currentArg[2], currentArg[3]); could use a comment block
  • Do not use console.log in production code
  • Also, read about apply, it allows you to call a function and pass an array of arguments so you don't have to be limited to 3 arguments.

    if (_srwc !== undefined) {
      for (var i=0; i < _srwc.length; i++){
        var currentArg = _srwc[i];
        if (typeof window[currentArg[0]] == 'function') {
          window[currentArg[0]](currentArg[1], currentArg[2], currentArg[3]);
        } else {
          console.log('The Setter method: "' + currentArg[0] + '" is undefined');
        }
      }
    } 
    
share|improve this answer
2  
2 spaces? Infidel! 4 spaces is the one true style for perfect legibility. Well, maybe we can compromise on three. And we can both agree that tabs are worse ;-) –  amon Mar 28 at 12:52
1  
Agreed on tabs!! –  konijn Mar 28 at 12:54
    
Philistines, the lot of you. Tabs forever!! :P –  cHao Mar 28 at 14:27
    
"It is more common to compare _srwc to undefined itself" <-- well, that's actually true, but note that on older environments undefined was overridable, so checking typeof is a better choice IMHO - also note that common isn't ever better :) –  LightStyle Mar 28 at 17:50
add comment

Use guard clauses, consistent indentation, some spaces around the operators and remove the double semicolon. You could also extract out an explanatory variable for currentArg[0] (which would remove some duplication and describe its content).

if (typeof _srwc == 'undefined') {
    return;
}
for (var i = 0; i < _srwc.length; i++) {
     var currentArg = _srwc[i];
     // TODO: just guessing, you might have a better name
     var methodName = currentArg[0]; 
     if (typeof window[methodName] != 'function') {
         console.log('The Setter method: "' + methodName + '" is undefined');
         continue;
     }
     window[methodName](currentArg[1], currentArg[2], currentArg[3]);
}

I suppose its a function or it could be extracted out to a function. Otherwise, the return won't be appropriate at the beginning.

share|improve this answer
1  
This could die without a continue in the if (typeof window[currentArg[0]] != 'function') { since you would be trying to execute currentArg without it being a function –  konijn Mar 28 at 12:53
    
@konijn: Thanks, fixed! –  palacsint Mar 28 at 12:54
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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