Skip to content

Conversation

@VachaganHambardzumyan
Copy link
Collaborator

No description provided.

@githuboftigran githuboftigran self-assigned this Oct 15, 2019
@githuboftigran githuboftigran self-requested a review October 15, 2019 18:22
var isPrime = true;
for (j = 2; j < startPrimeNum; j++) {
if (startPrimeNum % j === 0) {
isPrime = false;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loop will do 998 iterations, if startPrimeNum is 1000.
But after first iteration it's clear, that 1000 is not prime, so it will be better to break; the loop once the condition is satisfied.

@@ -1,38 +1,36 @@
var num = parseInt(prompt('Enter a natural number or "exit"'));
var num = prompt('Enter a natural number or "exit"');
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first glance it's not clear why this line is outside the loop. It's a bit confusing. If this line goes into top of while loop, the code will be clearer.

primeArr[j] = primeArr[j + 1];
primeArr[j + 1] = c;
for (var i = 0; i < primeArr.length - 1; i++) {
for (var j = 0; j < primeArr.length - 1; j++) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see bubble sort implementation here, but both loops run from 0 to primeArr.length, so primeArr.length^2 operations will be performed. But if first loop starts at i, instead of 0, only (primeArr.length * (primeArr.length + 1)) / 2 operations will be performed, which is much better.

for (var j = 2; j < k; j++) {
if (k % j === 0) {
return false;
var count = 1;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These numbers are confusing at first glance. Why 1 ? Why [2], why 3 ? I see your intention, but still, this is confusing. Please set count to 0, primeArray to empty array ([]) and startPrimeNum to 2. BTW add a comment to this line like:
var startPrimeNum = 2; // The first prime number
so it will be clear what this number is about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants