2
\$\begingroup\$
Array.prototype.itemsWithQuantity = function(n) {
    var counts = {};
    var result = [];
    var i = this.length;
    while(i--){
        counts[this[i]] = counts[this[i]] == undefined ? 1 : counts[this[i]] + 1;       
    }
    i = this.length;
    var alreadyPushed = {};
    while(i--){
        if(counts[this[i]] == n && !alreadyPushed[this[i]]){
            alreadyPushed[this[i]] = true;
            result.push(this[i]);
        }       
    }
    return result;
};

Above is my function that returns all the items in an array with a certain quantity. E.g., [1,2,3,2,3,4].itemsWithQuantity(2) == [2,3].

Not only is the code kinda ugly, but I also have a feeling there's a more efficient way to do it that I'm not spotting.

\$\endgroup\$
2
\$\begingroup\$

I guess you could use Array.forEach() to do the counting and laterArray.filter() to get the elements with the desired number of occurences.

It reduces the bookkeeping to the minimum of counts, goodbye alreadyPushed. It also reduces the boilerplateness a little, by giving you local variables implicitely:

  • You have this[i] everywhere, which can look a little cryptic. You could store it in a local var element = this[i]; which could be considered more readable.
  • in my suggested code, you get those local variables as a result of using functions that receive the value as a parameter which you can of course name however you want.

I think it depends on how much you are used to seeing certain patterns show up in code. After doing it for a while, you know that

var i = this.length;
while(i--){ //...}

will iterate over all the elements. Array.forEach() calls it by its name, which can improve readability.

I had my first ever go at the built in scripting thingy here on stackexchange, take a look and try the code:

Array.prototype.itemsWithQuantity = function(n) {
  var counts = {};

  // count
  this.forEach(function(element) {
    counts[element] = counts[element] == undefined ? 1 : counts[element] + 1
  });

  // return those elements (which are the keys(=property names) of the object) occuring n times
  return Object.keys(counts).filter(function(element) {
    return counts[element] == n;
  });
};

var input = [1, 2, 3, 3, 4, 1, 2, 5];
var output = input.itemsWithQuantity(2);

document.getElementById("output").innerHTML = output.toString();
<p id="output"></p>

\$\endgroup\$
  • \$\begingroup\$ Instead of forEach, you could use reduce. \$\endgroup\$ – Thomas Junk Apr 23 '16 at 13:48
  • \$\begingroup\$ @I'll add comments tomorrow Concerning "... counts[element] == undefined ? ..." : My first thought was to use JavaScript's in-operator: "elements in counts[element]". Strangely nobody took that approach. So I ask myself if it would have some disadvantages which I don't know? Or would it work the same? \$\endgroup\$ – michael.zech Apr 25 '16 at 8:45
  • \$\begingroup\$ @st88 no idea, I sticked to the way I assign the property. I didn't really think about in to be honest. \$\endgroup\$ – I'll add comments tomorrow Apr 25 '16 at 17:45
2
\$\begingroup\$

You can (ab-)use the short-circuit logic of Javascript to make the last line a little more readable:

counts[this[i]] = (counts[this[i]] || 0) + 1;
\$\endgroup\$
  • \$\begingroup\$ When you use a for-in loop with an array, you iterate over every property of the object, which will include methods attached by extending the prototype. It would be a problem here if I ever passed in the parameter 1. And it would be poor practice even if I wasn't passing in a parameter of 1, since it would perform extra iterations. \$\endgroup\$ – Jonah Apr 22 '16 at 18:57
  • \$\begingroup\$ But your second point is good, thanks :) \$\endgroup\$ – Jonah Apr 22 '16 at 18:57
  • \$\begingroup\$ Ugh, you're right. Haven't touched Javascript in a while and forgot that array iterators work on array indices instead of array elements (as they usually do in other languages). They don't iterate over all object properties as you claim though (just tested in Firefox). \$\endgroup\$ – David Foerster Apr 23 '16 at 12:09
1
\$\begingroup\$

I would use Javascripts ability to reduce Arrays (see MDN).

function itemByFrequency(arr, freq){
    function countFrequencies(o,n){
      o[n] = o[n] || 0;
      o[n] += 1;
      return o;
    };
    var fr = arr.reduce(countFrequencies, {});
    function filterFrequencies(o,n){
        if (fr[n]==freq) o.push(n);
        return o;
    }
    return Object.keys(fr).reduce(filterFrequencies, []);
}

Here is an example glot.io

\$\endgroup\$
  • 1
    \$\begingroup\$ I went through the same thought process. The problem is that it does check for minimum occurances, not exact ones. If you have more than n, it's still part of the result. I forked your code. To fix this, you'd have to remove the element from result again in case of o[n] == freq +1, which requires searching through result. This makes the approach less intuitive (imo) and harder to read, which is too high of a price for me to pay for single-loopness. Maybe it's even premature optimization. \$\endgroup\$ – I'll add comments tomorrow Apr 23 '16 at 12:57
  • \$\begingroup\$ Yes, you're absolutely right. I changed the code to my first intended version using reduce (twice) - Athough the last one could be using filter ;) \$\endgroup\$ – Thomas Junk Apr 23 '16 at 13:46
0
\$\begingroup\$

One could combine string-methods with array-methods. So that the used loops are reduced.

function getElementsWithCount(arr, requiredCount) {
  var arrAsString = arr.join(', ');
  var ret = [];
  var alreadyChecked = [];

  for (var i = 0; i < arr.length; i++) {
    // If the element is already in one of the two
    //   collector-arrays then continue.
    if ( ret.indexOf(arr[i]) !== -1 || 
         alreadyChecked.indexOf(arr[i]) !== -1 ) {
      continue;
    }

    var regExp =
        new RegExp('(^|\\s)' + arr[i] + '($|,)', 'g');

    // Compare the occurences within the string with 
    // demanded count.
    arrAsString.match(regExp).length === requiredCount ? 
      ret.push(arr[i]) : 
      alreadyChecked.push(arr[i]);
  }

  return ret;
}

var field = [1,2,3,2,3,4];

console.log( getElementsWithCount(field, 2) );

The for-loop is necessary because I want to use continue. Which isn't possible with forEach.

\$\endgroup\$
  • \$\begingroup\$ Note that you can use return; inside a forEach in the same way you are using that continue; inside your for \$\endgroup\$ – Pevara Apr 25 '16 at 13:24
  • \$\begingroup\$ @Pevara Sometimes one doesn't realize the simplest option although it's obvious. ;) Thanks a lot. \$\endgroup\$ – michael.zech Apr 25 '16 at 13:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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