2

I wrote a script to remove any null, undefined, 0, false or empty string value using .splice(), but the code has only removed NaN and 0s.

Here is what I tried:

function remove(arr){ // input [NaN, 0, 15, false, -22, '',undefined, 47, null]
    var bin = [];
    for (var i =0; i<arr.length; i++){
        if (arr[i] == (NaN || 0 || false || "" || undefined || null)){
            arr.splice(arr[i],1);
        }
    }
  console.log(arr); // Expected output [15, -22, 47]
}
Mohamed Hegazy
  • 285
  • 3
  • 18
  • 3
    `arr = arr.filter(e => e);` – Tushar Mar 18 '16 at 04:24
  • 2
    `arr = arr.filter(Boolean)` is built-in ES5. – dandavis Mar 18 '16 at 04:25
  • Possibly duplicate http://stackoverflow.com/questions/31925323/how-to-filter-out-nan-null-0-false-in-an-array-js – brk Mar 18 '16 at 04:41
  • Where did you find the syntax `x == (a || b || c || d)` and what do you expect it to do? Why are you trying to use `splice` instead of using `filter` to create a new array? –  Mar 18 '16 at 04:53
  • I was trying to check if x is equal to a or b or c or d, I can tell from your comment that it's wrong syntax, would you please explain why? I was using splice because I am new to programming so I didn't know any better – Mohamed Hegazy Mar 23 '16 at 03:10

5 Answers5

3

Problem in code:

  1. Iterate over array from last element to first element
  2. Array#splice expects first argument as index, not the item itself
  3. To check if an element is NaN, use isNaN().
  4. To compare with multiple values, compare the elements individually with each item

Working code:

function remove(arr) {
    for (var i = arr.length - 1; i >= 0; i--) {
        if (isNaN(arr[i]) || arr[i] === 0 || arr[i] === false || arr[i] === "" || arr[i] === undefined || arr[i] === null) {

        // Which is same as
        // if (!arr[i]) {
            arr.splice(i, 1);
        }
    }

    console.log(arr); // Expected output [15, -22, 47]
}

remove([NaN, 0, 15, false, -22, '', undefined, 47, null]);

Use Array#filter

arr = arr.filter(e => e);


var input = [NaN, 0, 15, false, -22, '',undefined, 47, null];

var filteredArr = input.filter(e => e);
document.body.innerHTML = '<pre>' + JSON.stringify(filteredArr, 0, 4) + '</pre>';

Or as said by @dandavis in comment

arr = arr.filter(Boolean);


var input = [NaN, 0, 15, false, -22, '',undefined, 47, null];

var filteredArr = input.filter(Boolean);
document.body.innerHTML = '<pre>' + JSON.stringify(filteredArr, 0, 4) + '</pre>';
Community
  • 1
  • 1
Tushar
  • 85,780
  • 21
  • 159
  • 179
  • look closer, it does appear that mutation is desired, so splice() might be needed still. – dandavis Mar 18 '16 at 04:36
  • @dandavis Didn't understand your last, but added the code using `splice` – Tushar Mar 18 '16 at 04:45
  • @Tushar Thanks alot for your answer, I have tried to run your code but only changed the iteration to count from the first element to last but it didn't work, whereas your original method worked, would you please explain why ascending looping will not work? – Mohamed Hegazy Mar 23 '16 at 03:29
  • @MohamedHegazy Ascending order will not work because when used `splice` the array will be updated and the `index` on the next element will not work. See [this fiddle](https://jsfiddle.net/a26av8dn/), in the console some elements(`0` and `-22`) are missed. – Tushar Mar 23 '16 at 03:31
  • @dandavis Mutation is not really desired, I was just solving this exercise so I thought splice would do the job – Mohamed Hegazy Mar 23 '16 at 04:10
2

This solution may not be that efficient 

function filterArray(el) {
    return (typeof (el) === "number" && el != 0 && !isNaN(el));
}

arr = [NaN, 0, 15, false, -22, '', undefined, 47, null];
console.log(arr.filter(filterArray));

jsfiddle

Tushar
  • 85,780
  • 21
  • 159
  • 179
brk
  • 48,835
  • 10
  • 56
  • 78
2

use lodash.

console.log(_.compact(arr));
Eric Hartford
  • 16,464
  • 4
  • 33
  • 50
1

You refer to using splice so I assume you want an in-place (mutating) solution. Here's a simple, quick approach:

function compact(a) {
  var i = 0, j = 0;

  while (i < a.length) {
    var v = a[i++];
    if (v) a[j++] = v;
  }

  a.length = j;
}

In case you care (and you probably shouldn't), this is 10 times faster than a loop over splice. See http://jsperf.com/compacting-an-array.

If you want a solution which creates a new array, the alternatives given in other answers using filter are fine, but here's a basic version:

function compact(a) {
  var i = 0, j = 0, result = [];

  while (i < a.length) {
    var v = a[i++];
    if (v) result[j++] = v;
  }

  return result;
}
  • Mutation means you change the input. That's what `splice` does, and that's how my first sample code works. The alternative to mutation (in-place) is to create a new array with just the values you want. That's how `filter` works, and also how my second sample code works. –  Mar 23 '16 at 05:07
0
 function bouncer(arr) {
  var newArr=arr.slice(0);
  var n=0;
  for(var i=0; i<arr.length;i++){
    if(arr[i]==false||arr[i]==null||arr[i]==0||arr[i]==""||arr[i]==undefined||Number.isNaN(arr[i])==true){
      newArr.splice(n,1);

    } else n++;
  }
  console.log(newArr);
  return newArr;
}

bouncer([null, NaN, 1, 2, undefined,0,'a']);

I have tried to do it with slice()... For NaN I used the function Number.isNaN() and if returns true goes on... The input was [null, NaN, 1, 2, undefined,0,'a'] and returned [ 1, 2, 'a'] as it is expected... Hope it helped you

Víctor Cruz
  • 1
  • 1