current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Join the Stack Overflow Community
Stack Overflow is a community of 6.5 million programmers, just like you, helping each other.
Join them; it only takes a minute:
Sign up
up vote 1 down vote favorite

I have a function that has been made to manipulate and create an object array and add a position key to each item within the array, but it seems to not be working properly at all...

My function:

var fillQueue = function(choices, currentQueue) {
  var choice, i, positionInQueue, previousTitle, resultQueue;

  resultQueue = [];
  previousTitle = "";

  i = 0;

  while (i < 10) {
    positionInQueue = i + 1;
    console.log('Adding song to position ' + positionInQueue);

    if (currentQueue[i]) {
      previousTitle = currentQueue[i].title;
      currentQueue[i].position = positionInQueue;
      resultQueue.push(currentQueue[i]);
    } else {
      choice = choices[Math.floor(Math.random() * choices.length)];

      if (choice.title !== previousTitle) {
        previousTitle = choice.title;
        choice.position = positionInQueue;
        resultQueue.push(choice);
      } else {
        choice = choices[Math.floor(Math.random() * choices.length)];
        previousTitle = choice.title;
        choice.position = positionInQueue;
        resultQueue.push(choice);
      }
    }

    i++;
  }

  return resultQueue;
};

If this is called correctly, substituting in values for choices and currentQueue (currentQueue can also be an empty array) like below, the function returns an array, which is intended, but screws around with the position keys on each object within it for some reason.

var test = fillQueue([ {title: '1'}, {title: '2'}, {title: '3'}, {title: '4'} ], [ { title: '5', position: 1 } ]);

The above variable will contain this:

[ { title: '5', position: 1 },
  { title: '1', position: 9 },
  { title: '3', position: 7 },
  { title: '1', position: 9 },
  { title: '2', position: 10 },
  { title: '2', position: 10 },
  { title: '3', position: 7 },
  { title: '1', position: 9 },
  { title: '1', position: 9 },
  { title: '2', position: 10 } ]

As you can see, it's not adding the positions correctly into each object. Each object within the returned array should have a position of i + 1, but instead it seems to be some random number from 1 to 10 - some of the objects even have the same position.

I have tried:

  • Renaming position to something else, in case it was something that was already being used by JavaScript
  • Making sure that the correct position is being added to the object before and after .pushing it into the array.

This is making me really confused. Your help would be appreciated.

Fiddle: https://jsfiddle.net/deansheather/jnw8jdf4/

share|improve this question
    
what is the output format you need exactly? – Taj Ahmed Feb 18 at 6:40
1  
What you are pushing into the array is a reference to a choice, so each time you update choice.position, all instances of that choice get the same "position" value, e.g. all the choices with "Title:1" will have the same position because they all reference the same object. You need to create a copy of the choice, update its position and push the copy into the array. – RobG Feb 18 at 6:43
    
@TajAhmed I am looking for an array containing 10 objects. Each object in this array must have a position property equal to i + 1. – deansheather Feb 18 at 6:44
    
@RobG How do I fix this then and change it from a reference into it's own separate variable? – deansheather Feb 18 at 6:45
up vote 1 down vote accepted

You are pushing references into the array, so each time you update choice.position all references to the same choice in the array get the same position value.

To fix that, copy the choice object, update its position and push that into the array, e.g.

if (choice.title !== previousTitle) {
        previousTitle = choice.title;
        newChoice = objCopyShallow(choice); // see below for copy function
        newChoice.position = positionInQueue;
        resultQueue.push(newChoice);
}

For this application, a simple copy function is:

function objCopyShallow(obj) {
  return Object.keys(obj).reduce(function(acc, v) {
      acc[v] = obj[v];
      return acc;
    },{});
}

There are a million questions and answers here about "how to copy an object" if you want something deeper.

share|improve this answer
up vote 1 down vote

As RobG Already said you are pushing references to your array.So create a copy of your choice object and then push to your array. create a copy of your object using clone() change your assignment to choice = clone(choices[Math.floor(Math.random() * choices.length)]); you are done.

function clone(obj) {
    if (null == obj || "object" != typeof obj) return obj;
    var copy = obj.constructor();
    for (var attr in obj) {
        if (obj.hasOwnProperty(attr)) copy[attr] = obj[attr];
    }
    return copy;
}
share|improve this answer
up vote 0 down vote

I was able to come up with the following. This works as I'm avoiding using for/while loops which have reference "gotchas" in JS you need to be aware of when updating values using it.

var fillQueue = function(choices, currentQueue) {
  // concat choices to end of currentQueue
  // create a new array instance with updated position property using .map
  // keep 10 elements from new array starting at index 0 
  return currentQueue.concat(choices).map( (e, i) => {
    return Object.assign({title:e.title, position:i+1});
  }).splice(0, 10)
};

share|improve this answer
    
This has the same issues as the OP. – RobG Feb 18 at 6:57

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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