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.

Given a hashmap (or a "Dictionary") as a JavaScript object, I'd like to get the Key who has the maximum Value - assuming all Values are integers.

In case there's more than one, I don't care which one of them.

Considering

var b = { '1': 9, '2': 7, '3': 7, '4': 9, '5': 3 };

I can extract the desired Key by:

parseInt(_(b).chain().pairs().max(function(p){return p[1];}).value()[0])

which returns 1.

How can this be achieved more elegantly?

I tried with _.invert as well but couldn't make it look better.

share|improve this question

closed as unclear what you're asking by 200_success Jan 31 at 12:43

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

    
It looks like you're seeking the key with the largest value. Note that the keys are strings while the values are integers. –  David Harkness May 18 '14 at 11:24
    
You're right, my apologies. I modified my question, but this still does not answer. –  user40171 May 18 '14 at 12:30
    
There's something ambiguous here, since there are 2 keys with a value of 9: b['1'] and b['4']. You say you want the "first" key ('1'), but JS objects are technically unordered, so you could get the '4' key instead. So do you want the lowest key with the highest value, or simply any key with the highest value? –  Flambino May 18 '14 at 13:23
    
"In case there's more than one, I don't care which one of them." - so I want simply any key with the highest value. –  user40171 May 18 '14 at 13:41
1  
If you use parseInt, you should always specify a base: parseInt(…, 10). –  Ingo Bürk May 18 '14 at 16:59

3 Answers 3

I don't know about elegant, but you can omit the initial _(), use + to convert to number instead of parseInt and just pull out the iterator.

var iterator = function(p){return p[1];}
var maxKey = +_.chain(b).pairs().max(iterator).value()[0];
// Removed the | 0 since the keys are integers to begin with

Here's an alternate way to do it using ES5 Array.prototype.reduce and Object.keys. You can also use underscore's _.reduce and _.keys for compatibility. Note that this runs through the entire array, so any duplicates found will use the latest value.

var max = Object.keys(b).reduce(function(max,key){
  return (max === undefined || b[key] > b[max]) ? +key : max;
});

Making it more clear what we're doing, in case you aren't familiar of reduce, is to use Array.prototype.forEach (or _.each):

var max;
Object.keys(b).forEach(function(key){
  max = (b[key] > b[max]) ? +key : max;
});
share|improve this answer
    
I think | 0 is an anti-pattern as it is completely unclear what it's doing to anyone who hasn't seen it before. What's wrong with parseInt? –  Ingo Bürk May 18 '14 at 16:58
    
I have to agree with @IngoBürk, |0 may be good if you're trying to Golf the code, but here we seek readability... However, I loved the idea of using an iterator function rather than using an anoynmous function. I just hoped someone will enlighten me, supplying a completely different approch for this task, avoiding the pairing and un-pairing etc. –  user40171 May 18 '14 at 17:48
1  
@IngoBürk most of this underscore stuff should be completely unclear to anyone who hasn't seen it before. Does that make it an antipattern also? –  Dagg May 18 '14 at 19:42
1  
@user40171 "here we seek readability" ... just looking at your line of code from the POV of someone who didn't write it, is it obvious that it is meant to return the key of the highest value in an object's properties? IOW is it really that readable to you? I usually find these densely-packed functional sorts of snippets to be difficult to read at a glance after they're written, sort of like regex. I think I'd be able to discern the purpose of an eqivalent for loop much faster. I'm curious what others think, though. –  Dagg May 18 '14 at 19:48
1  
You're missing typeof for max === 'undefined' and the final expression will not find a key if all values are negative. Use var max = null and max === null || ... –  David Harkness May 18 '14 at 20:03

The way I would do this with the current iteration of underscore (1.6) would be through reduce. Note use _.reduceRight if you want to favour items to the left

_.reduce(b, function(max, current, key) {
    return max && max.value > current ? max : {
        value: current,
        key: key
    };
}).key

However, in lodash you can use findKey which would be more intuitive. _.findKey will be available in underscore 2.0 if they accept my pull request #1587

var maxValue = _.max(b);
_.findKey(b, function(x) {
    return x === maxValue;
});
share|improve this answer
up vote 0 down vote accepted

Ok I think I found what I was looking for. Thanks anyway for all help.

parseInt( _(b).invert()[_(b).max()] )

This returns 4, which is just good as 1 according to requirements. It looks more elegant and it is way shorter than the pairing stuff.

share|improve this answer
1  
This may match the new requirements, but it doesn't answer the original question. Feel free to leave it, but please leave the question as-is and accept the best answer to it when you like. –  David Harkness May 18 '14 at 20:12
    
What "new requirements"? The requirements never changed. The statement "In case there's more than one, I don't care which one of them" was always in the body of question. I will accept my own answer only if no one else suggests more elegant / readable way to do it. –  user40171 May 19 '14 at 5:56
1  
This returns the max value, but the question asked how to get the key of the max value. Not a big deal, though. :) –  David Harkness May 19 '14 at 15:11

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