CODEWITHSUNDEEP
javascriptarraysfor-loopwhile-loop
Davtho1983
Davtho1983

Reputation: 3954

JavaScript Prime checker not working

My prime checker is not working. I think there are simpler ways to do this challenge, but the functionality I need is:

For a given number p:

1) Fill an array numArray with numbers from 2 to p

I have achieved this with: 

for (var i=2; i<(p + 1); i++) {
    numArray.push(i);
  };

2) While numArray is not empty:

My code doesn't use a while loop because I'm not sure how to use one to iterate through the array, but a for loop isn't working either?

CAN PEOPLE JUST RESPOND WITH SUGGESTIONS, NOT COMPLETE FIXES. Definitely please please do not write complete code that finds prime numbers. I'm more interested to know how I can iterate through an array that I keep slicing depending on elements of the array, and any other pointers about how to structure my code.

Also I'm confused as to why 

(numArray[k] % prime == 0)

seems to miss numbers?

function checkIfPrime(p) {
  numArray = [];
  primeArray = [];

  for (var i = 2; i < (p + 1); i++) {
    numArray.push(i);
  };

  for (var j = 0; j < numArray.length; j++) {
    if (primeArray.indexOf(numArray[j]) === -1) { //if numArray of j is not already in primeArray

      var prime = numArray[j];
      console.log(prime + " is not in numArray")

      primeArray.push(prime);
      console.log(primeArray);
      numArray.splice(j, 1);
      console.log(numArray);


      for (var k = 0; k < numArray.length; k++) {
        if (numArray[k] % prime == 0) {
          numArray.splice(k, 1);
        };
      }

    }

  };

}


p = 5;
console.log(checkIfPrime(p));

Upvotes: 1

Views: 67

Answers (2)

Rajesh
Rajesh

Reputation: 24945

Since you are more interested in suggestions than working code, lets start with with alternatives.

Issue is, when you splice an item from array, it changes its length. Nothing new in it, but the issue is, you are iterating over same array.

So for a given array [2, 3, 4, 5], if j is 0, prime will 2. So you remove 2 and 4. So the new length is 2. Issue is the remaining elements will be shifted. So it would look like [3, 5].

Now in the iteration, j will be 1. So it will start from 5 and will skip 5.

Alternative:

Instead of removing elements, assign them to a default value that will be skipped by default. This way, your index will remain correct and your logic will be simple.

function checkIfPrime(p) {
  var numArray = [];
  var primeArray = [];

  for (var i = 2; i < (p + 1); i++) {
    numArray.push(i);
  };

  for (var j = 0; j < numArray.length; j++) {
    if (numArray[j] !== 0 && primeArray.indexOf(numArray[j]) === -1) { //if numArray of j is not already in primeArray
      var prime = numArray[j];

      primeArray.push(prime);

      for (var k = 0; k < numArray.length; k++) {
        if (numArray[k] % prime == 0) {
          numArray[k] = 0;
        };
      }
    }
  };
  console.log(primeArray)
}


p = 5;
console.log(checkIfPrime(p));

splice every multiple of prime from numArray

The simplest way is to use Array.filter. If you manipulate array while looping itself, it will complicate logic. Also, an object should be immutable. So, you can loop over array and create a temp array with necessary values and replace the value.

This can be done using array.filter or even using for.

numArray = numarray.filter(function(num){
  return num % prime !== 0;
})

Assuming numArray will have possible prime numbers, we can try to minimize the obvious non-candidates.

var numArray = [2, 3];

for (var i = 8; i<=p; i++) {
  if(i % 2 !== 0 || i % 3 !== 0) {
    numArray.push(i)
  }
}

Also, since prime numbers are fixed, we can assume a length and pre-compute list of prime numbers. If user enters a bigger number, you just have to compute numbers from limit to new limit. This way you are saving some processing time:

var numArray = [2, 3];

function computePrimes(limit) {
  var init = numArray[numArray.length - 1] || 0;
  for (var i = init; i<= limit; i++) {
    if(isPrime) {
      numArray.push(i);
    }
  }
}

function isPrime(num) {
  for (var i = 3; i< num; i++) {
    if(num % i === 0)
      return true;
  }
  else false;
}

computePrimes(100);

function getPrimeNumbers(num) {
  if(num > numArray[numArray.length - 1]) {
    computePrimes(num);
  }
  // ... Your computation logic
}

Upvotes: 2

iCart
iCart

Reputation: 2351

As Mike said, it's generally a bad idea to use a for-loop to iterate through an array while modifying it.

I see two ways to go about it:

Either manage your iteration variables manually and make sure it increments only when it needs to .

Or use the array's filter method. Here's a basic example to get you started:

[1,2,3].filter(function(x){return x != 2})
// Will return [1,2]

Keep in mind that this will return a new array with filtered values, so you'd have to reassing it by doing something like numArray = numArray.filter(...)

Upvotes: 1

Related Questions