2
\$\begingroup\$

How can I improve the following for loop (wannabe function) to create harmonic sizes for headers based on a specific ratio?

var base = 16;
var goldenRatio = 1.618;
var list = "";
for(var i=6; i>0; i--){
    list = "\nh" + i + " { font-size: " + base + "px; }" + list;
    base *= Math.round(base*goldenRatio);
}   
console.log(list)
\$\endgroup\$
1
\$\begingroup\$

This is what your code produces and I'm wondering if this really what you want:

"
h1 { font-size: 1.0957612059345655e+45px; }
h2 { font-size: 2.602367950332107e+22px; }
h3 { font-size: 126822144384px; }
h4 { font-size: 279968px; }
h5 { font-size: 416px; }
h6 { font-size: 16px; }"

The numbers are huge. Looking at this page, it seems you should not have base inside the multiplication here:

base *= Math.round(base*goldenRatio);

Also, by rounding at this step, you lose precision. It would be better to round right before you put the value in the string. Like this:

var base = 16;
var list = "";
for (var i = 6; i > 0; i--) {
    list = "\nh" + i + " { font-size: " + Math.round(base) + "px; }" + list;
    base *= goldenRatio;
}   

I'm also wondering if you really want the \n at the start of the resulting string. It would seem more natural to put it at the end of each line, like this:

list = "h" + i + " { font-size: " + Math.round(base) + "px; }\n" + list;

Notice also that I added more spaces around the operators in the for. There is no real standard for this in JavaScript, but this is a practice I borrow from other languages where it is a standard, for improved readability. Compare these two versions, I hope you'll agree that this is an improvement:

for(var i=6; i>0; i--){

for (var i = 6; i > 0; i--) {
\$\endgroup\$
  • 1
    \$\begingroup\$ Hi Janos, thank you for correcting the code, and, yes, that's what I want, copying it from console, instead of using base = base*1.618 I refactored it a little bit to better present it on Code Review getting the opposite result. :) In any case, is there a better, more elegant way, to get that same result or the for loop is (was) acceptable? I felt it could have been improved someway. Thank you again for your time. \$\endgroup\$ – caruso_g Oct 16 '14 at 19:08
  • \$\begingroup\$ I added one more point about formatting. Other than what I mentioned, I don't really see further possible improvement in this short piece of code. \$\endgroup\$ – janos Oct 16 '14 at 19:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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