Skip to main content
rm signature
Source Link
svick
  • 24.3k
  • 4
  • 52
  • 89

If I may give you one tip right away: try to write out the variable names and don't use abbreviations. If primeFactorCounter seems too long to you, just say counter. Right now your code is short and you know what pfCountermeans, but it is just hard to read for someone else or for you when you read it again in 2 Weeks.

By the way nicely documented, it was really easy to understand your code that way :)

Anyway, back to your question.

  • the first for loop fills an array with the natural numbers. This is not necessary. the numbers won't change, so instead of looping through the array every time to get the number, just use the loop's index for(int i=1; i<=n; i++) where nwould be the 10000 instead of the array's length.

  • another little tweak to speed things up could be to stop the loop when you find out that you have no use for the number. But you would have to change the loops conditions a little bit. You only need to test the numbers that are smaller than the one you are investigating and bigger than 1 (because every number can be divided by one or itself):

so instead of:

for (int j = i; j > 0; j--) {
  if (numbers[i] % j == 0) {
     pfCounter++;
  }
}

try

for (int j = i-1; j > 1; j--) {
  if (numbers[i] % j == 0) {
     break;   // this breaks out of the loop
  }
}

Now you don't have to test if your pfCounter is bigger than 1. (PS: I didn't replace the numbers[i] by i just start the "i-loop" with i=1and you'll be fine!

  • regards, Christoph

If I may give you one tip right away: try to write out the variable names and don't use abbreviations. If primeFactorCounter seems too long to you, just say counter. Right now your code is short and you know what pfCountermeans, but it is just hard to read for someone else or for you when you read it again in 2 Weeks.

By the way nicely documented, it was really easy to understand your code that way :)

Anyway, back to your question.

  • the first for loop fills an array with the natural numbers. This is not necessary. the numbers won't change, so instead of looping through the array every time to get the number, just use the loop's index for(int i=1; i<=n; i++) where nwould be the 10000 instead of the array's length.

  • another little tweak to speed things up could be to stop the loop when you find out that you have no use for the number. But you would have to change the loops conditions a little bit. You only need to test the numbers that are smaller than the one you are investigating and bigger than 1 (because every number can be divided by one or itself):

so instead of:

for (int j = i; j > 0; j--) {
  if (numbers[i] % j == 0) {
     pfCounter++;
  }
}

try

for (int j = i-1; j > 1; j--) {
  if (numbers[i] % j == 0) {
     break;   // this breaks out of the loop
  }
}

Now you don't have to test if your pfCounter is bigger than 1. (PS: I didn't replace the numbers[i] by i just start the "i-loop" with i=1and you'll be fine!

  • regards, Christoph

If I may give you one tip right away: try to write out the variable names and don't use abbreviations. If primeFactorCounter seems too long to you, just say counter. Right now your code is short and you know what pfCountermeans, but it is just hard to read for someone else or for you when you read it again in 2 Weeks.

By the way nicely documented, it was really easy to understand your code that way :)

Anyway, back to your question.

  • the first for loop fills an array with the natural numbers. This is not necessary. the numbers won't change, so instead of looping through the array every time to get the number, just use the loop's index for(int i=1; i<=n; i++) where nwould be the 10000 instead of the array's length.

  • another little tweak to speed things up could be to stop the loop when you find out that you have no use for the number. But you would have to change the loops conditions a little bit. You only need to test the numbers that are smaller than the one you are investigating and bigger than 1 (because every number can be divided by one or itself):

so instead of:

for (int j = i; j > 0; j--) {
  if (numbers[i] % j == 0) {
     pfCounter++;
  }
}

try

for (int j = i-1; j > 1; j--) {
  if (numbers[i] % j == 0) {
     break;   // this breaks out of the loop
  }
}

Now you don't have to test if your pfCounter is bigger than 1. (PS: I didn't replace the numbers[i] by i just start the "i-loop" with i=1and you'll be fine!

Source Link

If I may give you one tip right away: try to write out the variable names and don't use abbreviations. If primeFactorCounter seems too long to you, just say counter. Right now your code is short and you know what pfCountermeans, but it is just hard to read for someone else or for you when you read it again in 2 Weeks.

By the way nicely documented, it was really easy to understand your code that way :)

Anyway, back to your question.

  • the first for loop fills an array with the natural numbers. This is not necessary. the numbers won't change, so instead of looping through the array every time to get the number, just use the loop's index for(int i=1; i<=n; i++) where nwould be the 10000 instead of the array's length.

  • another little tweak to speed things up could be to stop the loop when you find out that you have no use for the number. But you would have to change the loops conditions a little bit. You only need to test the numbers that are smaller than the one you are investigating and bigger than 1 (because every number can be divided by one or itself):

so instead of:

for (int j = i; j > 0; j--) {
  if (numbers[i] % j == 0) {
     pfCounter++;
  }
}

try

for (int j = i-1; j > 1; j--) {
  if (numbers[i] % j == 0) {
     break;   // this breaks out of the loop
  }
}

Now you don't have to test if your pfCounter is bigger than 1. (PS: I didn't replace the numbers[i] by i just start the "i-loop" with i=1and you'll be fine!

  • regards, Christoph