My mission is to solve this:
Write a function that will take an array of numbers, and will return the average of all numbers in the array that are prime number.
So far I wrote this:
function average(a){
var arr=[];
var newarr=[];
var i,j,sum,sum1,average,counter,counter1;
counter=counter1=sum=sum1=0;
for(i=0;i<a.length;i++){
for(j=2;j<(Math.floor(a[i]/2));j++){
if((a[i]%j===0) && (a[i]>2)){
arr.push(a[i]);
break;
}
}
}
for(i=0;i<a.length;i++){
sum=sum+a[i];
counter++;
}
for(i=0;i<arr.length;i++){
sum1=sum1+arr[i];
counter1++;
}
average=(sum-sum1)/(counter-counter1);
return average;
}
var a=[88,44,32,30,31,19,74,169,143,109,144,191];
console.log(average(a));
I am allowed to use only: conditions (if
), loops, --
, ++
, %
, /
, *
, -
, +
, ==
, =!
, =>
, >
, =<
, <
, ||
, &&
, =%
, =/
, =*
, +-
, =+
, array.length
, array.pop()
and concat
.
Any suggestions? Feedback on what I wrote? Thank you!
1 Answer 1
Single responsibility
Try to organize your programs in a way that every function has a single responsibility. For example checking if a number is a prime or not stands out here as a clear example that should be in its own function:
function isPrime(num) {
var factor;
for (factor = 2; factor < Math.floor(num / 2); factor++) {
if (num % factor == 0) {
return false;
}
}
return true;
}
Simplify the logic
The code implements the following algorithm:
- Build an array of non-primes
- Compute the sum of all values
- Compute the sum of non-primes
- Subtract the sum of non-primes from the sum of all numbers, and divide this by the count of non-primes
Consider the simpler alternative:
- Compute the sum of primes, and also count them
- Return the sum of primes divided by their count
This alternative is matches well the problem description too, so it's easy to understand. And so is the code.
function average(nums) {
var i;
var sum = 0;
var count = 0;
for (i = 0; i < nums.length; i++) {
if (isPrime(nums[i])) {
sum += nums[i];
count++;
}
}
return sum / count;
}
Use better names
The code is very hard to read because most of the variable names don't describe well their purpose, for example:
arr
stores non-prime numbers ->nonPrimes
would describe thisa
is an array of numbers ->nums
would describe thisnewarr
... is not even used -> should not be there- And so on! (
counter1
,sum1
are poor names too, more on that later
Avoid unnecessary array creation
The code used arr
to collect non-primes,
to compute their sum in a next step.
You could compute the sum directly, without building an array for it.
Creating arrays consumes memory, and therefore can be expensive, and can make your program inefficient. When there is an easy way to avoid it, then avoid it.
Use whitespace more generously
This is hard to read:
for(i=0;i<a.length;i++){
This is much easier to read, and a widely used writing style:
for (i = 0; i < a.length; i++) {
=!
,=+
and many others are not operators in JavaScript. Can you please double-check the description? (I suspect these should be!=
and+=
, respectively. There are several other odd ones too.) Also, I assume you can also usefunction
, since you are in fact using it. Right? \$\endgroup\$