Nested forEach loops unexpected behavior

1.1k views Asked by At

In my poker app I have an array of hands, each hand being array of randomly selected card objects with value and suit:

[ [ { value: 5, suit: 's' },
    { value: 4, suit: 's' },
    { value: 6, suit: 'c' },
    { value: 11, suit: 'd' },
    { value: 12, suit: 'c' } ],
  [ { value: 9, suit: 'd' },
    { value: 12, suit: 'h' },
    { value: 8, suit: 'c' },
    { value: 12, suit: 's' },
    { value: 2, suit: 's' } ],
  [ { value: 4, suit: 'h' },
    { value: 6, suit: 's' },
    { value: 10, suit: 'c' },
    { value: 3, suit: 'd' },
    { value: 7, suit: 'd' } ] ]

To prepare the hands for evaluation I want to return an array of hand objects, each with an array of values and suits. So the output would be:

[ 
  { 
    values: [5, 4, 6, 11, 12],
    suits: ['s', 's', 'c', 'd', 'c'] 
  },      
  { 
    values: [9, 12, 8, 12, 2], 
    suits: ['d', 'h', 'c', 's', 's'] 
  },
  { 
    values: [4, 6, 10, 3, 7],
    suits: ['h', 's', 'c', 'd', 'd'] 
  } 
]

I'm trying to use nested forEach loops to achieve this, like so:

let temp = []
hands.forEach((el) => {
  temp = el
  el = {}
  el.values = []
  el.suits = []
  temp.forEach((obj) => {
    el.values.push(obj.value)
    el.suits.push(obj.suit)
    console.log(el) //expected output
  })
  console.log(el) //expected output
})
console.log(hands) //same as original

However as the comments outline, it behaves as expected until the loop has finished, where hands has not changed at all. What am I missing here?

5

There are 5 answers

5
Jamiec On BEST ANSWER

forEach does not change the array on which it was called, it simply iterates the array calling a function for each element. If you want to build a new array inside that function, just do so

var newArray = [];
hands.forEach((el) => {
   var newValue = // do something here
   newArray.push(newValue);
});

I think what you're trying to do is this:

var hands = [ [ { value: 5, suit: 's' },
    { value: 4, suit: 's' },
    { value: 6, suit: 'c' },
    { value: 11, suit: 'd' },
    { value: 12, suit: 'c' } ],
  [ { value: 9, suit: 'd' },
    { value: 12, suit: 'h' },
    { value: 8, suit: 'c' },
    { value: 12, suit: 's' },
    { value: 2, suit: 's' } ],
  [ { value: 4, suit: 'h' },
    { value: 6, suit: 's' },
    { value: 10, suit: 'c' },
    { value: 3, suit: 'd' },
    { value: 7, suit: 'd' } ] ];

let newValues = []
hands.forEach((el) => {
  var temp = {values:[], suits:[]};
  el.forEach((obj) => {
    temp.values.push(obj.value)
    temp.suits.push(obj.suit)
    
  })
  newValues.push(temp);
})
console.log(newValues) // NOT same as original

There is many ways to achieve this same thing, the best I could come up with is below - and avoids the double hand.map seen in other answers (which is pretty inefficient).

var hands = [ [ { value: 5, suit: 's' },
    { value: 4, suit: 's' },
    { value: 6, suit: 'c' },
    { value: 11, suit: 'd' },
    { value: 12, suit: 'c' } ],
  [ { value: 9, suit: 'd' },
    { value: 12, suit: 'h' },
    { value: 8, suit: 'c' },
    { value: 12, suit: 's' },
    { value: 2, suit: 's' } ],
  [ { value: 4, suit: 'h' },
    { value: 6, suit: 's' },
    { value: 10, suit: 'c' },
    { value: 3, suit: 'd' },
    { value: 7, suit: 'd' } ] ];

let newValues = hands.map(h => {
      return h.reduce( (p,c) => {
            p.values.push(c.value);
            p.suits.push(c.suit);
            return p;
        },{values:[], suits:[]});
  });

console.log(newValues) // NOT same as original

0
Quentin On
hands.forEach((el) => {

You assign each object in the array, in turn, to the variable el.

el = {}

You overwrite the variable el with a reference to a new object.

el is no longer connected to its original value.

You are just replacing the value of el. The properties in the array are still pointing to the original objects.


To change the objects in the original array, you need to assign a new object to them explicitly.

hands.forEach((originalValue, index, array) => {
  // originalValue is what you used to call `temp`
  // Just make `el` a new object
  var el = {};
  // Put it in the array
  array[index] = el;
  // Then continue as before
3
Mike 'Pomax' Kamermans On

Let JavaScript do this for you instead of writing lots of code, by taking advantage of the map function that arrays support:

let rewritten = data.map(hand => {
  return {
    values: hand.map(card => card.value),
    suits: hand.map(card => card.suit),
  } 
});

(with the outer { return and } in the arrow function being unnecessary if you put it all on the same line, but for the purposes of an answer that makes it far less readable of course)

This takes your original data, which is already and array sized on hands (three in this case), and then applies a transform to each hand. Specifically, it creates a new object where the array of "values and suits" have been turned into an array of "just values", using map, and an array of "just suits", also using map.

Arrays in JavaScript come with a number of utility functions for packing/unpacking/iterating/transforming, and it's worth giving those a read through. They allow you to do big things with very little code.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array is worth a read if you've not read up on the API for arrays in a while, or ever. It also explains how to use recude for instance, which can be used to make this code more efficient (see Jamiec's answer above for that).

0
Nina Scholz On

You could map the values directly in an object for the properties.

var data = [[{ value: 5, suit: 's' }, { value: 4, suit: 's' }, { value: 6, suit: 'c' }, { value: 11, suit: 'd' }, { value: 12, suit: 'c' }], [{ value: 9, suit: 'd' }, { value: 12, suit: 'h' }, { value: 8, suit: 'c' }, { value: 12, suit: 's' }, { value: 2, suit: 's' }], [{ value: 4, suit: 'h' }, { value: 6, suit: 's' }, { value: 10, suit: 'c' }, { value: 3, suit: 'd' }, { value: 7, suit: 'd' }]],

result = data.map(a => (
  { 
    values: a.map(b => b.value), 
    suits: a.map(b => b.suit) 
  })
);

console.log(result);
.as-console-wrapper { max-height: 100% !important; top: 0; }

You could use a less iteration style.

var data = [[{ value: 5, suit: 's' }, { value: 4, suit: 's' }, { value: 6, suit: 'c' }, { value: 11, suit: 'd' }, { value: 12, suit: 'c' }], [{ value: 9, suit: 'd' }, { value: 12, suit: 'h' }, { value: 8, suit: 'c' }, { value: 12, suit: 's' }, { value: 2, suit: 's' }], [{ value: 4, suit: 'h' }, { value: 6, suit: 's' }, { value: 10, suit: 'c' }, { value: 3, suit: 'd' }, { value: 7, suit: 'd' }]],
    keys = ['value', 'suit'],
    result = data.map(
        a => a.reduce(
            (r, b) => (keys.forEach(k => r[k + 's'].push(b[k])), r), { values: [], suits: [] }
        )
    );

console.log(result);
.as-console-wrapper { max-height: 100% !important; top: 0; }

0
kevin ternet On

You just need to create an object with two properties (values and suits) before the second loop and inject him in the result after this second loop like this :

var hands = [[{ value: 5, suit: 's' }, { value: 4, suit: 's' }, { value: 6, suit: 'c' }, { value: 11, suit: 'd' }, { value: 12, suit: 'c' }], [{ value: 9, suit: 'd' }, { value: 12, suit: 'h' }, { value: 8, suit: 'c' }, { value: 12, suit: 's' }, { value: 2, suit: 's' }], [{ value: 4, suit: 'h' }, { value: 6, suit: 's' }, { value: 10, suit: 'c' }, { value: 3, suit: 'd' }, { value: 7, suit: 'd' }]];
var output = [];
hands.forEach(x => {
  var temp = {values: [], suits: []};
  x.forEach(y => {
    temp.values.push(y.value);
    temp.suits.push(y.suit);
  })
  output.push(temp);
});
console.log(output);