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.

I forgot to mention using the result of bar to calculate foo's value in the question I asked here.

The function bar only needs to be called if conditionA is true. But when the result of bar (resultBar) is falsy, I want to run the code that would have run if conditionA had been false. Is there a more concise way of approaching this without duplication?

var scale = 1;
if(settings.doUseOutlines) {
    var typefaceSize = getTypefaceSize(typefaceComp);
    if(typefaceSize) {
        scale = parseFloat(settings.fontScale) / typefaceSize[1];
    } else {
        scale = parseFloat(settings.fontScale) / typefaceComp.height; // duplicated
    }
} else {
    scale = parseFloat(settings.fontScale) / typefaceComp.height;
}
share|improve this question
1  
As per the FAQ, please show real code and not psuedocode with fake variables. –  Winston Ewert Feb 14 '13 at 16:01
    
Updated. Thanks. –  Michael Delaney Feb 14 '13 at 17:28

2 Answers 2

up vote 4 down vote accepted

You can use the short circuit feature of the && operator to only call the function if the first condition is true, and the fact that the value of the assignment is the assigned value so that you can do the assignment and check it's value in one statement:

var scale, typefaceSize;
if (settings.doUseOutlines && (typefaceSize = getTypefaceSize(typefaceComp))) {
  scale = parseFloat(settings.fontScale) / typefaceSize[1];
} else {
  scale = parseFloat(settings.fontScale) / typefaceComp.height;
}

This is of course a bit of a convoluted way to write it. The original code is still clearer in some ways, so you get to choose between simplicity and avoiding repetition.

Choosing the height depending on the conditions and thus breaking out most of the calculation (based on Winston Ewerts answer), makes it even less repetetive:

var height = typefaceComp.height, typefaceSize;
if (settings.doUseOutlines && (typefaceSize = getTypefaceSize(typefaceComp))) {
  height = typefaceSize[1];
}
var scale = parseFloat(settings.fontScale) / height;
share|improve this answer
1  
I choose your first suggestion because I like assigning variables only once. Versus assigning height twice if the condition is true. Thanks. –  Michael Delaney Feb 14 '13 at 19:52
    
@MichaelDelaney - I'd suggest putting a comment in next to that piece of code explaining what it does, since it is convoluted. –  Bobson Feb 14 '13 at 20:05
var height = typefaceComp.height;
if(settings.doUseOutlines) {
    var typefaceSize = getTypefaceSize(typefaceComp);
    if(typefaceSize) {
        height = typefaceSize[1];
    } 
} 

var scale = parseFloat(settings.fontScale) / height;
share|improve this answer
    
That's an improvement, I'd move the 'var' statement for resultBar to the top, Also, adding a 'falsy' value may lead to unexpected results and should be avoided in most cases. Like your comment suggested OP should show real code. –  Benjamin Gruenbaum Feb 14 '13 at 16:04
1  
@BenjaminGruenbaumm, why would you move the var statement to the top? –  Winston Ewert Feb 14 '13 at 16:07
    
Apologies for not reading the FAQ before hand. I updated the code with real code. I wasn't very good at abstracting my question. –  Michael Delaney Feb 14 '13 at 17:30
    
@WinstonEwert because of variable hoisting javascript moves the var statement to the top regardless of our code style. Javascript has (currently) no block scope. I think (like a lot of others like Douglas Crockford) that keeping the var statement there creates the illusion of block scope as well as causing unexpected results (for example when hiding a global before the variable was 'declared' in the 'if'). –  Benjamin Gruenbaum Feb 14 '13 at 17:36
    
@BenjaminGruenbaum, I see your point; however, I don't see that its worth the clutter introduced by following it. –  Winston Ewert Feb 14 '13 at 17:46

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.