Categorising a number in JavaScript

Question

I've written a function that takes a number and returns a value depending on where that number lies on a pre-defined scale. That's probably a little confusing so I've put the code below and added a JSFiddle (although you might be better off in the dev console).

var getSize = function(number){
        var sizes = {
            100 : 'url_t',
            240 : 'url_s',
            320 : 'url_n',
            500 : 'url_m',
            640 : 'url_z',
            800 : 'url_c',
            1024 : 'url_l',
        },
        result = sizes[100];

    for (var size in sizes){
        if(number < size){
            result = sizes[size];
            break;
        }
        result = sizes[1024];
    }
    return result;
};
getSize(10) // returns url_t
getSize(130) // returns url_s
getSize(2000) // returns url_l
getSize(600) // returns url_z

It works and it's fairly performant, it's just a bit icky. I've tried to come up with a better solution but it just feels ickier. Is there anything I'm missing that would make this a bit nicer? I was playing about with putting the sizes in an array so they're a little more JSON-ish, but then I got lost.

Answer 1

You might not need to loop through an object, and a simple switch statement would suffice.

function getSize(number) {
    switch (true) {
        case number < 100: return "url_t";
        case number < 240: return "url_s";
        case number < 320: return "url_n";
        case number < 500: return "url_m";
        case number < 640: return "url_z";
        case number < 800: return "url_c";
        default:           return "url_l";
    }
}

As a side note, I've seem people use switch (true) before when they want to use some logic in their case's instead of just hard coded values. Kind of a nice alternative to an if-else construct, and would probably perform better than looping over an object using a for-in loop.

You could still keep the sizes in an object:

var sizes = {
    100 : 'url_t',
    240 : 'url_s',
    320 : 'url_n',
    500 : 'url_m',
    640 : 'url_z',
    800 : 'url_c',
    1024 : 'url_l'
};

function getSize(number) {
    switch (true) {
        case number < 100: return sizes[100];
        case number < 240: return sizes[240];
        case number < 320: return sizes[320];
        case number < 500: return sizes[500];
        case number < 640: return sizes[640];
        case number < 800: return sizes[800];
        default:           return sizes[1024];
    }
}

Edit: A problem arises if you need to change the sizes for your application. That's why I'm leaning towards the first function, which just a switch statement and hard coded values.

Answer 2

• Your for..in construct should have an Object.hasOwnProperty check in the extremely unlikely event that somebody decides to abuse prototypes (and on Object, no less!).
• Your indentation is a little inconsistent.
• You assign to the result variable excessively.
• Some values are hard-coded, but they shouldn't be. At the very least, move them into a constant instead of leaving them as magic numbers.

You could try something like the following:

var getSize = function(number) {
    var MAX_SIZE = 1024;

    var result;
    var sizes = {
        100: 't',
        240: 's',
        320: 'n',
        500: 'm',
        640: 'z',
        800: 'c',
        1024: 'l'
    };

    for (var size in sizes) {
        if (sizes.hasOwnProperty(size)) {
            if (number < size) {
                result = sizes[size];
                break;
            }
        }
    }
    return "url_" + (result || sizes[MAX_SIZE]);
}

Incidentally, is it intentional that getSize(100) will return url_s instead of url_t? If not, just change the < above to a <=.

---

Revision. Better yet, we can use Object.keys(). This has the advantage of being entirely unreliant on magic numbers; we can instead just change our sizeMap object as needed. Furthermore, this deals with @megawac's concern about the undefined enumeration of for...in.

var getSize = function(number) {
    var result;
    var sizeMap = {
        100: 't',
        240: 's',
        320: 'n',
        500: 'm',
        640: 'z',
        800: 'c',
        1024: 'l'
    };

    var sizes = Object.keys(sizeMap).sort(function(a, b) {
        return a - b;
    });

    for (var i = 0; i < sizes.length; i++) {
        if (number < sizes[i]) {
            result = sizeMap[sizes[i]];
            break;
        }
    }
    return "url_" + (result || sizeMap[sizes[sizes.length - 1]]);
}

Answer 3

Alright first thing the order that an engine enumerates object properties is not guaranteed!!! Its pretty consistent but there are engine bugs and you shouldn't rely on the behaviour.

That said, this is how I would write your getSize. You can use Object.keys(sizes) to create the ranges list but again - order isn't gaurenteed. You could sort it I suppose?

var getSize = (function() {
    var sizes = {
        100 : 'url_t',
        240 : 'url_s',
        320 : 'url_n',
        500 : 'url_m',
        640 : 'url_z',
        800 : 'url_c',
        1024 : 'url_l',
    };
    // Note you can do Object.key(sizes); to create this list
    var ranges = [100, 240, 320, 500, 640, 800, 1024];

    return function(number) {
        for (var i = 0, len = ranges.length; i < len; i++) {
            var key = ranges[i];
            if (number < key) {
                return sizes[key];
            }
        }
        // otherwise return the largest size
        return sizes[key];
    };
})();

It may be overkill but I also created the sizes and ranges object once

Answer 4

I'd go with a variation on Greg's excellent answer, but I would invert the sizes map to eliminate magic numbers:

var sizes = {
    url_t : 100,
    url_s : 240,
    url_n : 320,
    url_m : 500,
    url_z : 640,
    url_c : 800,
    url_l : 1024
};

function getSize(number) {
    switch (true) {
        case number < sizes.url_t: return 'url_t';
        case number < sizes.url_s: return 'url_s';
        case number < sizes.url_n: return 'url_n';
        case number < sizes.url_m: return 'url_m';
        case number < sizes.url_z: return 'url_z';
        case number < sizes.url_c: return 'url_c';
        default:           return 'url_l';
    }
}

Or, instead of the switch, to avoid the repetition of its case lines:

var names = ['url_t', 'url_s', 'url_n', 'url_m', 'url_z', 'url_c', 'url_l'];

function getSize(number) {
    for (var i in names) {
        var name = names[i];
        if (number < sizes[name]) {
            return name;
        }
    }
    return names[names.length - 1];
}

And another thing, keep in mind that a , right before the closing } is incorrect JavaScript and may not work in all browsers and environments:

// incorrect:
var sizes = { 1024 : 'url_l', }

// correct:
var sizes = { 1024 : 'url_l' }

Answer 5

I won't use the for in structure.

It's loops thorught the properties of the object and also the properties of his prototype.

If that's your only choice you have to use the hasOwnProperty for every size you check to avoid posible errors.

I would use something like:

var sizes = {
 values: [100,240,320,500,640,800,1024],
 names: ['url_t', 'url_s', 'url_n', 'url_m', 'url_z', 'url_c', 'url_l']
},

and then the usual for loop.