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.

Good morning, I started programming 5 months ago.

I created a function to calculate the font size to fit the width of the website I'm doing.

I believe the code is a bit ugly and could use some improvement by creating a more logical function, faster and less heavy.

Can someone look at the code and comment on it for my own improvement.

Thanks to all Agredeço.

//Script jquery

$(window).resize(function() { 
  newWidthGhost = $('#ghostFontes').width();
  widthFontsResize();
});

$(document).ready(function(){
    $('body').find('*').addClass('wFonte');

$('.wFonte').each(function(i, el){
    fontHeight[i] = $(el).css('font-size');
});

oldWidthGhost = $('#ghostFontes').width(); // TAMANHO WIDTH;

widthFontsReady();

});

function widthFontsResize(){
    $('.wFonte').each(function(i, el){
        nFonte['e'+i] = parseInt((newWidthGhost * nFonte[i]) / oldWidthGhost);
        $(el).css({'font-size': nFonte['e'+i]+'px'});
});

}

function widthFontsReady(){
    $('.wFonte').each(function(i, el){
        nFontHeigth[i] = parseInt(fontHeight[i].substr(0,2));
        nFonte[i] = parseInt((oldWidthGhost * nFontHeigth[i]) / 1586);
        $(el).css({'font-size': nFonte[i]+'px'});
});

}
share|improve this question
add comment

2 Answers 2

up vote 2 down vote accepted

From what I understand from your code, you are scaling the original fonts as the window resizes, am I correct?

For a totally better approach, use the native em scaling technique. That is, change the parent em measurement, you also change the children within it. Here's sample code which is exponentially shorter than yours:

$(function() {
        //caching frequently used elements and values
    var ghostFontes = $('#ghostFontes'),
        $window = $(window),
        originalWidth = ghostFontes.width();

    $window.on('resize', function() {
        //scale the parent container's em measurement 
        //with the ratio of width to original width
        ghostFontes.css('font-size',(ghostFontes.width() / originalWidth)+'em');

    });
});​
share|improve this answer
add comment

Use strict-mode

Start your code with "use strict";:
http://stackoverflow.com/questions/1335851/what-does-use-strict-do-in-javascript-and-what-is-the-reasoning-behind-it

Don't pollute the global namespace

When you don't use var x; to declare a variable, it is implicitly declared in the global namespace. If another script does the same, both scripts will be using the same variable.

It is not very easy for this to happen with an odd name like newWidthGhost, but fontHeight sounds like a better candidate for these surprises.

Run the code through JSLint

Run your code through JSlint, and try to understand the errors and suggestions it gives you.

That alone will greatly improve the quality of your code.

share|improve this answer
    
I understand, but I'm wanting more of a technical code, where I can improve, logic, where I can slow down or I can use to improve or be a little less heavy. Because the code puts it in a class all tags (wFonte). As I said at the beginning'm new and want to learn better what I did: D. But still I appreciate the tip that is also very important –  user1875730 Dec 4 '12 at 16:38
    
I hope you get the advice you seek. Also: I understand, but I think it's important to first improve the foundations, before working on improving the house. –  ANeves Dec 4 '12 at 17:31
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.