0

I'm writing a small script in JavaScript to output an array. I want to use the 'years' array, push the data into another array called new_array, and output it via console.log().

The only problem is, this is the output! enter image description here

years = [1996, 1994, 1981, 1976];
new_array = [];


for(i = 0; i <= years.length; i++) {
    popped_element = Object.values(years.pop([i]));
    new_array.push(popped_element);
    console.log(new_array)

}
TheRealMrCrowley
  • 976
  • 7
  • 24
Wali Chaudhary
  • 167
  • 3
  • 14
  • the undefined is due to the = in <= years.length. – WBT Feb 05 '17 at 01:39
  • 2
    Why are you using `Object.values`? – Sebastian Simon Feb 05 '17 at 01:40
  • 1
    What are you trying to achieve? – guest271314 Feb 05 '17 at 01:42
  • 1
    Are you really just trying to `Array.reverse()`? That already exists. `years.pop([i])`? What is that supposed to do? `Object.values()` takes an Object as an argument. – StackSlave Feb 05 '17 at 01:42
  • Ah, you're right. The reason I was using it was because before I thought push returned an integer value, and I would need to convert it to an array to represent the value. But I just misused it in my code after toying around for a while! – Wali Chaudhary Feb 05 '17 at 01:47
  • You're jumping ahead to writing code when the specifications are unclear. Given the `years` array that your program starts with, what is the output that you want it to produce? – Michael Geary Feb 05 '17 at 01:47
  • I wanted to output '[1976, 1981, 1994]' . Sorry for the lack of clarity, my understanding of JS is quite weak since I just started learning today. – Wali Chaudhary Feb 05 '17 at 01:51
  • Welcome to Stack Overflow and Javascript! Feel free to check out [these tips](http://stackoverflow.com/help/how-to-ask) for getting good answers in the future. – WBT Feb 05 '17 at 01:53
  • 1
    *Array.prototype.pop* doesn't take any arguments, they're ignored. It always returns the last element of the array. – RobG Feb 05 '17 at 02:29

5 Answers5

6

You're over complicating it, you don't use both a for loop and pop unless your iterating from the end to the beginning

if you iterate from the beginning while popping the end, you will only get half the array:

 years = [1996, 1994, 1981, 1976];
        new_array = [];


        for(i = 0; i < years.length; i++) {
          new_array.push(years.pop());

        }
          console.log(new_array)

you have two options if you are modifying the original array

1) Use A While Loop

keep looping as long as there are still elements in the original array

years = [1996, 1994, 1981, 1976];
    new_array = [];
    
    
while(years.length) {
  new_array.push(years.pop());

}

  console.log(new_array)

2) Use a Decreasing For Loop

iterate from the end of the loop to the beginning

years = [1996, 1994, 1981, 1976];
new_array = [];


for(i = years.length - 1; i >= 0; i--) {
  new_array.push(years.pop());

}
  console.log(new_array)
TheRealMrCrowley
  • 976
  • 7
  • 24
  • 1
    Reversing the count order on i in the first snippet does not change anything about how the code works, but makes it less standard, more different from the original, and more likely to be a source of a bug in the future. – WBT Feb 05 '17 at 01:56
  • it in no way makes it more likely for a bug and reverse loops are not against standard. you don't modify the array you're woking on while looping through it in increasing order if you're poping off the end. otherwise you would end on the second iteration – TheRealMrCrowley Feb 05 '17 at 01:58
  • 1
    if you're counting iterations going from `0` to length and removing items from the end towards the beginning, your beginning items will never be `pop`ed – TheRealMrCrowley Feb 05 '17 at 02:01
  • Just save the length before entering the loop. `for(var len = array.length; len; len--)`! – ibrahim mahrir Feb 05 '17 at 02:06
  • ibrahim, then you have to do math every iteration. you start at the index before the length, same as in an increasing loop you end at the index before the length – TheRealMrCrowley Feb 05 '17 at 02:07
  • 1
    fair, forgot we were popping, but if your gonna do that, just use a while loop – TheRealMrCrowley Feb 05 '17 at 02:09
  • This answer need a bit of tidying and it would be perfect. – ibrahim mahrir Feb 05 '17 at 02:11
  • @ibrahimmahrir what should be tidied? – TheRealMrCrowley Feb 05 '17 at 02:11
  • Just get rid of the first example of using `for`. Keep just the `while` solution and try to explain more why the third example is wrong (try and use **bold text** as titles). – ibrahim mahrir Feb 05 '17 at 02:13
  • 1 UP anyway. Because your answer is the only one that points out that the length property is changing. – ibrahim mahrir Feb 05 '17 at 02:15
  • @ibrahimmahrir thanks, it blows my mind how many don't realize that, and that one person even tried to correct me – TheRealMrCrowley Feb 05 '17 at 02:20
  • I didn't realize it either. I know `pop` and `shift` actually reduce the size of the array by 1 every time they're called. But it's hard to actually notice it (never used them inside a loop before). So bravo! – ibrahim mahrir Feb 05 '17 at 02:26
  • How could I edit this solution to add only every 500ms a new item? I tried with timeout but somehow did not act as I imagined and only waits for the first item but then pops all together. – Oliver Feb 23 '20 at 15:57
4

Maybe you really want to do this?

var years = [1996, 1994, 1981, 1976];
var reverse_years = years.slice().reverse(); // years.slice() creates new instance of Array so .reverse() does not affect original Array
console.log(reverse_years);

If you don't care if the original Array is affected:

var years = [1996, 1994, 1981, 1976];
years.reverse(); console.log(years);
StackSlave
  • 10,613
  • 2
  • 18
  • 35
  • 1
    He doesn't need the original array afterwards (because it will be empty because of those `.pop` calls). So why use `.slice`. Just reverse it! – ibrahim mahrir Feb 05 '17 at 02:02
  • Sure, if you're not using years again you can simply do `years.reverse();` without even assigning it to anything, since the original Array is affected. – StackSlave Feb 05 '17 at 02:05
2

You don't say what the intention of the code is, but it has a few issues anyway:

for(i = 0; i <= years.length; i++) {
    popped_element = Object.values(years.pop([i]));

Object.values is experimental and not widely available, so you should not use that. pop doesn't take any arguments, so you pop the last value of years each time. And you're calling Object.values on the value returned by pop, which is a string that doesn't have any enumerable properties, hence the empty array.

If your intention is to return a copy of the array in reversed order, then copy the array and reverse it:

var years = [1996, 1994, 1981, 1976];

// Copy with slice, reverse with reverse
var reversed = years.slice().reverse();
console.log(reversed)
RobG
  • 142,382
  • 31
  • 172
  • 209
1

Just iterate and assign:

years = [1996, 1994, 1981, 1976];
new_array = [];


for(var i=0; i<years.length; i++) {
    new_array.push(years[i]);
    console.log(new_array);
}

then arrange elements in revere order (if that's what you wanted):

new_array.reverse();

and delete the items from years (again, if that's what you wanted):

years = [];

..or, just use slice to copy:

new_array = years.slice();
thebjorn
  • 26,297
  • 11
  • 96
  • 138
  • This takes more memory than OP's strategy [once the implementation is corrected]. – WBT Feb 05 '17 at 01:55
  • @WBT I think you might be under the misconception that `Array.pop()` reduces the memory used by the array. It may, but probably not. Array access is much faster than array mutation too... – thebjorn Feb 05 '17 at 02:01
  • this is actually the most efficient solution if you're just getting all elements – TheRealMrCrowley Feb 05 '17 at 02:23
  • though instead of looping from the beginning THEN reversing, just iterate from the end and reassign the old array. – TheRealMrCrowley Feb 05 '17 at 02:24
  • also if your array is referenced elsewhere and you need propagation, you need to do years.splice() instead of reassigning the original variable – TheRealMrCrowley Feb 05 '17 at 02:25
  • @TheRealMrCrowley You're right on all counts.I wasn't sure if the OP really wanted the list in reverse order, which is the major reason I have reverse as a separate operation. In other languages the reverse for-loop is littered with pitfalls (e.g. http://stackoverflow.com/questions/665745/whats-the-best-way-to-do-a-reverse-for-loop-with-an-unsigned-index), which is another reason I didn't use it ;-) – thebjorn Feb 05 '17 at 02:33
  • @thebjorn C is a whole different ball game though. in javascript you should be just as familiar with them as incrementive loops. there are tons of uses i've came across that make them more efficient than always starting at the beginning – TheRealMrCrowley Feb 05 '17 at 02:39
  • @TheRealMrCrowley I almost never use for-loops on indexes in Javascript anymore. `Array.forEach()`, `Array.map()`, and `Object.keys(obj).map(..)` usually express my intent much clearer. – thebjorn Feb 05 '17 at 02:42
  • i agree with map, but forEach creates it's own scope, and is much less efficient is almost every browser than a simple for loop, i rarely use it – TheRealMrCrowley Feb 05 '17 at 02:43
  • @TheRealMrCrowley I normally transpile from ES6, and forEach with arrow functions makes it much more convenient. It is quite a bit slower (https://jsperf.com/for-vs-foreach/37) – thebjorn Feb 05 '17 at 02:51
  • 1
    @TheRealMrCrowley—I agree with your sentiments, however Array methods are very much faster in recent browsers than they were to the extent that there is now no significant performance difference with *for* or other types of loop. Even [*DOM NodeLists*](https://dom.spec.whatwg.org/#interface-nodelist) are getting a *forEach* (`iterable;`). ;-) – RobG Feb 05 '17 at 02:52
  • i guess it's just a force of habit for me at this point. in js if i've gotta iterate, then for loop it is. – TheRealMrCrowley Feb 05 '17 at 02:56
  • idk, I'd call 63% slower significant. you won't see the difference in you day to day apps, but with large scale data its a big thing. also interestingly enough reverse loops are THE FASTEST because you don't have to check length after every iteration – TheRealMrCrowley Feb 05 '17 at 03:00
  • @TheRealMrCrowley yea, I saw that (the reverse loop speed) too. Interesting. – thebjorn Feb 05 '17 at 03:01
  • @TheRealMrCrowley—using `var i=0, iLen=arr.length; i – RobG Feb 05 '17 at 03:04
  • @RobG i said you won't see it in day to day, but having worked with large scale data, i can assure you there are times that it matters – TheRealMrCrowley Feb 05 '17 at 03:07
0

Remove the call to Object.values() in the first line within the loop to solve the empty-array problem:

popped_element = years.pop();

From MDN:

The Object.values() method returns an array of a given object's own enumerable property values.

Your years have no enumerable property values, so it's returning an empty array and adding that empty array as an element in new_array.

The undefined at the end is due to the = in <= years.length. To fix, make that loop line:

for(i = 0; i < years.length; i++) {

or better yet, store the length before the loop and compare to that instead of accessing years.length on each loop iteration because pop() changes the array length.

WBT
  • 2,249
  • 3
  • 28
  • 40
  • 1
    if you use years.pop() while iterating from the beginning, your loop is going to end in the middle. you're result is going to be `years = [1996, 1994];` new_array=[1976, 1981]`; – TheRealMrCrowley Feb 05 '17 at 02:02
  • "*Your years have no enumerable property values*" yes it does. The problem is that the OP is calling *Object.values* on `years.pop()`, so effectively `Object.values('1976')`. – RobG Feb 05 '17 at 02:43
  • @RobG You seem to be missing a key distinction in English between the singular **set** of years, which has properties, in contrast to the plural **years** themselves (e.g. '1976') which do not. I'm making the same point you're trying to say, but you misinterpreted that. – WBT Feb 05 '17 at 02:58
  • @wbt—I can certainly make that distinction, but it was in your head, not in the prose. ;-) – RobG Feb 05 '17 at 03:12
  • @RobG Look at my prose and the use of a plural verb "have," referring to the elements of an array. Look at your comment quoting that and your attempt to contradict it using the singular forms "it" and "does," apparently referring to the array itself. See the mismatch? ("It" in my next sentence refers to the *function,* `Object.values`.) – WBT Feb 05 '17 at 03:51