Skip to content

Instantly share code, notes, and snippets.

@iendjinn
Created January 31, 2017 13:36
Show Gist options
  • Save iendjinn/761615d418d32ceec63fcd0372eea310 to your computer and use it in GitHub Desktop.
Save iendjinn/761615d418d32ceec63fcd0372eea310 to your computer and use it in GitHub Desktop.
for (var i = 0; i < 100; i++) {
var denominator = intensiveFunction();
var numerator = asynchronousFunction(i);
var fractions = [];
if (i < 10) fractions.push(numerator / denominator);
else if (i < 30) fractions.push((numerator * 2) / denominator);
else if (i < 90) fractions.push((numerator / 2) / denominator);
else if (i < 100) fractions.push((numerator) / (2 * denominator));
}
for (var i = 0; i < 100; i++) {
console.log(fractions[i]);
}
/*
Optimisations:
Calculate denominator once outside of first for loop
Remove nested ifs and replace with 4 for loops
Bugs:
Fractions is out of scope in second for loop
numerator could be undefined, push should be in a callback to asynchronousFunction
*/
@TobyEalden
Copy link

  • Generally, I think it would be better to give a more concrete example rather than 'intensiveFunction' etc
  • Not having any evidence of callback support in 'asynchronousFunction' is too misleading, and anyway using an asynchronous function inside a for loop is a non-starter.
  • Even the single-line if statements should use braces (according to Toby and Douglas Crockford :) )
  • Use ES6 syntax? Otherwise I don't think 'fractions' is out of scope in second for loop (see variable hoisting)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment