CODEWITHSUNDEEP
javascriptarrays
user8758206
user8758206

Reputation: 2191

Pushing the values into an array results in undefined variables but works inside a function

I'm trying to only push the values in the 'eachNumber' array with the indexes from the 'indexes' variable inside the 'appearMost' array, but for some reason it returns an array with undefined values:

var indexes = [1,2];
var appearMost = [];
var eachNumber = [4, 7, 9, 8];

indexes.map(function(c) { appearMost.push(eachNumber[c]) }); // should return [7,9]

The result of appearMost should be [7,9].

Strange, because I've built a function that returns the number appearing most frequently in an array which relies on the above line that doesn't seem to work. For example:

mostAppearing([5,5,2,2,1]); // correctly returns 5
mostAppearing([3,4,1,6,10]); // correctly returns -1
mostAppearing([4,7,7,7,9,9,8]); // correctly returns 7
mostAppearing([4,7,7,9,7,9,9,8]); // correctly returns 9

And the function has the code:

function mostAppearing(arr) { // e.g. var arr = [4,7,7,9,7,9,9,8];

var eachNumber = Array.from(new Set(arr)); // [4, 7, 9, 8];

if (arr.length == eachNumber.length) {
    return -1;
} else {
    var counts = eachNumber.map(function(c){ return arr.filter(function(el){ return el==c }).length }); // [1, 3, 3, 1];
    var maxVolume = Math.max(...counts); // 3
    var volVolume = counts.filter((c) => c == maxVolume).length; // 2

    if (volVolume == 1) {
        return arr[maxVolume];
    } else {
        var indexes = counts.reduce((a, c, i) => (c === maxVolume) ? a.concat(i) : a, []); // [1,2]
        var appearMost = [];
        indexes.map(function(c) { appearMost.push(eachNumber[c]) }); // relies on this line
        return Math.max(...appearMost);
    }
}

}

Can anyone explain (1) why undefined values are the result rather than [7,9], and (2) how my function works correctly? It should fail. Thanks for any help here.

Upvotes: 2

Views: 81

Answers (3)

HMR
HMR

Reputation: 39360

To filter through the entire array for each item is probably not the most efficient.

You can go through the array once with a reduce creating a Map that has the array item as key and the amount it occurs as value.

Then reduce it once more getting the most occurring and highest number. I put the guard of empty array and edge case of all numbers only appearing once (return -1 in both cases) in a seperate function:

const highestMostAppearing = (arr) =>
  [
    ...arr
      .reduce(
        (result, number) =>
          result.set(number, (result.get(number) || 0) + 1),
        new Map(),
      )
      .entries(),//Map where key is the number and value is the amount of time it occurs
  ].reduce(//this will error with empty array but mostAppearing will guard for that
    //result is highestNumber and mostAppeared so far
    //  item is the number and how many times it appeared
    ([highestNumber, mostAppeared], [number, appears]) =>
      appears > mostAppeared//current item appeared more often than the most appeared so far
        ? [number, appears]//return current number and how many times it appeared
        //next line checks if current number appeared the same times as highest so far
        //  and checks if current number is higher than the highest appeared number
        : appears === mostAppeared && highestNumber < number
          ? [number, appears]//replace result with current item values
          : [highestNumber, mostAppeared],//return previous result (is most appearing and highest)
  );
const mostAppearing = (arr) => {
  if (arr.length === 0) return -1;//do not call highestMostAppearing with empty array
  const [highest, appearing] = highestMostAppearing(arr);
  if (appearing === 1) return -1;//all numbers appear only once (expensive op here)
  return highest;//return most appearing highest nubmber
};

console.log('=======', mostAppearing([5, 5, 2, 2, 1]));
console.log('=======', mostAppearing([]));

Upvotes: 0

Karan
Karan

Reputation: 12629

Update your code as below and you can get desired result. Here count holds value as object { data: d, count: d.length }. then max will hold maximum repeated value count. Then filtered counts object for maximum repeated value and selected only data to map in appearMost object. Returned max value from appearMost.

function mostAppearing(arr) { // e.g. var arr = [4,7,7,9,7,9,9,8];
  var eachNumber = Array.from(new Set(arr)); // [4, 7, 9, 8];

  if (arr.length == eachNumber.length) {
    return -1;
  } else {
    var counts = eachNumber.map(function(c) {
      var d = arr.filter(el => el == c);
      return { data: d, count: d.length }
    });

    var max = Math.max(...counts.map(x => x.count));
    var appearMost = counts.filter(c => c.count == max).map(x => x.data[0]);

    return Math.max(...appearMost);
  }
}

console.log(mostAppearing([5,5,2,2,1])); // correctly returns 5
console.log(mostAppearing([3,4,1,6,10])); // correctly returns -1
console.log(mostAppearing([4,7,7,7,9,9,8])); // correctly returns 7
console.log(mostAppearing([4,7,7,9,7,9,9,8])); // correctly returns 9

Upvotes: 1

Shushan
Shushan

Reputation: 1225

The value of appearMost is updated correctly. 

var indexes = [1,2];
var appearMost = [];
var eachNumber = [4, 7, 9, 8];

indexes.map(function(c) { appearMost.push(eachNumber[c]) })
console.log(appearMost)

I believe you expected the return value of the map function to be 7,9 instead of the value inside appearMost. The map itself will not return a value as you did not use return inside your function. A better practice would be having the map function return array instead of mutating an existing one:

appearMost = indexes.map(function(c) { return eachNumber[c] })

Upvotes: 3

Related Questions