Javascript code efficiency

Question

I'm not really sure if it's okay to ask this here. If not, just vote me down. But i'm looking for an opinion on the efficiency of my aspect ratio function. The following is a function to determine the aspect, and limit the size of an image. How did i do?

function constrainTwoNumbers(options){

d = {
    dir: 'either', // direction: 'auto', 'vertical' or 'horizontal'. What side of the image do you want to constrain?
    orgw:0,
    orgh:0,
    target:100,
}

// merge the options with the default values
o = $.extend(d, options);

// create object to write results into
var result = [];

switch(o.dir){
    case 'either':      
        // no direction is set, limit the largest side.
        // determine what the orientation is.
        if(o.orgw > o.orgh){ //landscape
            aspect = o.orgw / o.target;
        }else if(o.orgw < o.orgh){ //portrait
            aspect = o.orgh / o.target;
        }else if(o.orgw === o.orgh){ // the image is square. Just pass both dimensions as targeted
            result.w = o.target;
            result.h = o.target;
            return result;
        }                   
        break;
    case 'horizontal':      
            aspect = o.orgw / o.target;
        break;
    case 'vertical':            
            aspect = o.orgh / o.target;
        break;
}

result.w = Math.round(o.orgw / aspect);
result.h = Math.round(o.orgh / aspect);     
return result;
}

need to change your comment to say "either" instead of "auto". — Gabriele Petrioli
– Gabriele Petrioli, Commented Dec 10, 2010 at 9:50
Looks okay - are you actually having problems with the algorithm's speed? — Piskvor left the building
– Piskvor left the building, Commented Dec 10, 2010 at 9:54

Gabriele Petrioli · Accepted Answer · 2010-12-10 10:07:19Z

2

You could condense it to a single if/else

function constrainTwoNumbers(options){

    var d = {
        dir: 'either', // direction: 'either', 'vertical' or 'horizontal'. What side of the image do you want to constrain?
        orgw:0,
        orgh:0,
        target:100
    };

    // merge the options with the default values
    var o = $.extend(d, options);

    // create object to write results into
    var result = [];
    if ((o.dir === 'either' && o.orgw > o.orgh) || (o.dir === 'horizontal'))
    {
      aspect = o.orgw / o.target;
    }
    else
    {
      aspect = o.orgh / o.target;
    }

    result.w = Math.round(o.orgw / aspect);
    result.h = Math.round(o.orgh / aspect);     
    return result;
}

edited Dec 10, 2010 at 10:07

answered Dec 10, 2010 at 9:59

Gabriele Petrioli

197k35 gold badges275 silver badges331 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

powtac · Accepted Answer · 2010-12-10 09:59:54Z

1

You can use the http://code.google.com/closure/compiler/ to check for performance.

And http://jslint.com/ to look for memory leaks and other errors.

answered Dec 10, 2010 at 9:59

powtac

41.1k28 gold badges120 silver badges173 bronze badges

Collectives™ on Stack Overflow

Javascript code efficiency

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related