current community

more stack exchange communities

Stack Exchange
tour help
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.
up vote 1 down vote favorite

How would I make this code look less ugly?

// we are looping over an array of images
// width is a parameter passed to the current function 
//
// the resulting string looks like this: "30px 0"
// (it's a tool that returns coordinates for a CSS sprite)
// when iterating over the first item, we don't need 
// to add "-" and "px" to X coordinate
result.push(
    ( i === 0 ? "0" : "-" + i * ( ( is2PixelRatio ? width / 2 : width ) + 10 ) + "px" ) + " 0"
);
share|improve this question
add comment

2 Answers

up vote 3 down vote

Minor upgrades, nice idea for the prepop result Raynos

result = ["0 0"]; // Outside of loop

// ...

if (is2PixelRatio) width /= 2;   
width += 10; // [Insert magik number explanation here]

result.push(  -(i * width) + "px 0"  ); // Negative numbers will convert to "-30"
share|improve this answer
add comment
up vote 1 down vote 
// old

var result = [], width, is2PixelRatio;
for (var i = 0; i < 10; i++) {
    width = i;
    result.push(
        ( i === 0 ? "0" : "-" + i * ( ( is2PixelRatio ? width / 2 : width ) + 10 ) + "px" ) + " 0"
    );
}
console.log(result);

// new

var result2 = ["0 0"], width, is2PixelRatio = true;
for (var i = 1; i < 10; i++) {
    width = i;
    result2.push("-" + i*(width+10) + "px 0");
}
if (is2PixelRatio) {
    result2 = result2.map(function(val) {
        return val.replace(/[\-]\d+/, function(v) { 
            return v / 2;
        });
    });  
}
console.log(result2);

Basically inject the default value into the array at the start. Then do that if is2PixelRatio logic at the end on the entire array.

Live example.

Uses Array.prototype.map

share|improve this answer
 
while this is certainly prettier, and I love the idea of pre-populating array with "0 0" and removing that piece of logic from the loop, on iphone4 it will do 2 loops instead of one – and performance is really important in my case. –  gryzzly Jun 29 '11 at 13:06
 
@gryzzly sounds like a micro-optimisation. The algorithm is still O(n). Have you benchmarked and profiled the code? Have you identified this particular loop to be a real noticeable bottleneck? I'm a firm believer that you should only optimise algorithms to minimal O and should not care about O(n) vs O(2n) vs O(5n). Constants in the range of an order of magnitude are a different issue (i.e. O(20n) vs O(n) is significant) –  Raynos Jun 29 '11 at 13:14
 
Well, this may be true in general, but not in all cases, since not all methods are executed equally frequently. Difference between 10ms run and 20ms is most often negligible, except if you do happen to run that function 500 times ;) –  markos Aug 28 '11 at 9:37
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.