# Sum All Odd Fibonacci Numbers(Help to make code clean)

Tell us what’s happening:
Hey guys,
After scratching my head for almost a day, I found the solution to this one but my code is a mess.
Need suggestions on how can i clean this up.
Each and every feedback is appreciated.
Thank You

``````

function sumFibs(num) {
var newArr = [1,1];
for(let i=2;i<num;i++){
newArr[i] = newArr[i-1] + newArr[i-2];
}
var filteredArr = newArr.filter((x) => x%2 !== 0);
var sum =0;
for(let j=filteredArr.length;j>=0;j--){
if(filteredArr[j] <= num){
sum += filteredArr[j];
}
}
return sum;
}
sumFibs(4);

``````

User Agent is: `Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/68.0.3440.106 Safari/537.36`.

First, properly indent the code (see below) which will help with some of the readability. This includes using spaces in expressions like writing x % 2 !== 0 instead of x%2 !==0.

``````function sumFibs(num) {
var newArr = [1, 1];
for (let i = 2; i < num; i++) {
newArr[i] = newArr[i - 1] + newArr[i - 2];
}
var filteredArr = newArr.filter((x) => x % 2 !== 0);
var sum = 0;
for (let j = filteredArr.length; j >= 0; j--) {
if (filteredArr[j] <= num) {
sum += filteredArr[j];
}
}
return sum;
}
sumFibs(4);
``````

Now replace variables like i, x, and j with more meaningful names to describe what value they hold.

For example, in the line where you filter, you could use a parameter name like num for x.

``````var filteredArr = newArr.filter(num => num % 2 !== 0);
``````

Also, if you are going to use higher order functions like filter, take advantage of using them for other for loops when possible. For example, you could replace:

``````  var sum = 0;
for (let j = filteredArr.length; j >= 0; j--) {
if (filteredArr[j] <= num) {
sum += filteredArr[j];
}
}
return sum;
``````

with the following using reduce:

``````return filteredArr.reduce((sum, val) => sum + val <= num ? val : 0, 0);
``````

Lastly, stick with using the latest declaration keywords like let and const and do not throw var into the mix.

2 Likes

Thank you so much for your feedback and explanation.
@RandellDawson
One thing i really face issues with is reduce().
Could you explain that a little or suggest some resources for further reading??

Is it the reduce method or the ternary operator I used inside the reduce that you do not understand? Or is it both?

Point out which part you are having trouble with and I will try to explain what is happening.

Here are a couple resources on reduce:

1 Like

I get the ternary operators.
It’s the reduce that i do not get even though i have solved challenges related to it.
Initially i had issues with map() and filter() as well but i got hang of them. However i just, am not able to comprehend reduce().

Let me break down my use of reduce above:

``````return filteredArr.reduce((sum, val) => sum + val <= num ? val : 0, 0);
``````

The reduce method has two parameters. The 1st parameter is a callback function which will be called on each element in the array. The 2nd parameter (which is optional) is the initial starting value you want to use for the accumulator argument in the callback function.

The main arguments you will typically use in the callback function are the first (known as the accumulator) and the second (the current element being iterated over). You can name these arguments anything you want, but it makes your code more readable to use names which describe what the arguments contain.

In each iteration, you must always return the accumulator, so that is starts with the last value it had in the previous iteration. The final value of the accumulator is what will get returned to where it was originally called.

In my example, I knew I wanted to create a running total (a sum) of all the Fibonacci numbers contained in filteredArr, so I named the accumulator argument sum. Next, I named the 2nd argument val, because each element is number. Normally, I would have used num as the argument name, but since I already had a num variable which I needed to compare each number in the array to, I had to use a different name.

I used arrow function syntax to make the code more concise. In each iteration I used the ternary operator to check if the current val was less than or equal to num. If it was, I added val to sum, otherwise I added 0. The sum was calculated and returned implicitly at the end of each iteration.

You will notice the extra , 0 at the end of the reduce statement. Here, I used the optional 2nd parameter to start the original value of the accumulator (sum) to zero. If could have left this off, if I knew for sure that there would always be at least one number in the array, but in general I typically like to initialize this parameter. If filteredArr was empty and I did not initialize the accumulator to zero, it would error out.

1 Like

Thanks again @RandellDawson.
I will try to read from other resources as well and i will have to solve a few algorithms as well usign this to get hang of it.