Help with code

I'm new to this and trying to learn so can someone point me in the right direction without giving the answer away? What should I look up? How can I optimize the code / improve the logic?




1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
#include <iostream>
#include <vector>
using namespace std;

int main() {
	vector<int> nums;
	int a = 1;
	int b = 2;
	int c = 0;
	int MaxSize = 4000000;
	
	unsigned long total = 0;
	while (c < MaxSize && b < MaxSize && a < MaxSize){
		c = b + a;
		nums.push_back(c);
		a = c + b;
		nums.push_back(a);
		b = a + c;
		nums.push_back(b);
		
};
	int size = nums.size();
	cout << "\n\n";

	for (int i = 0; i < size; ++i){
		cout << "\n";
		if (nums[i] < MaxSize){
			cout << nums[i];
		}
		for (int i = 0; i < size; ++i){
			if (nums[i] < MaxSize){
				if ((i + 2) < size){
					total = total + nums[i + 2];
				}
				else{
					total = total + nums[i];
				}
				
				
			}
		}
	}
	cout << "Total is: " << total;
	cout << "\n\n\n";
	system("PAUSE");
	return 0;
}
Last edited on
It's hard to tell because we don't really know what the purpose of your program is? To me, it seems like you've just arbitrarily decided to add numbers and save them, then add up some of those saved numbers.

What are you actually trying to accomplish here?
It's a challenge on a website and I was trying to get help without giving away the specifics of the challenge. Even though it's not very challenging to most =). I'm trying to find the fibonacci numbers between 1 and 4million and then find the even ones and then add the even ones together.
This is Project Euler #2, right? (Sum of even Fibonacci numbers under 4 million)
Your original topic title was more descriptive...

Anyways, I see two things that are wrong with this code.

Number 1:
7
8
9
int a = 1;
int b = 2;
int c = 0;

Your starting values should really be a = 1 and b = 1; otherwise, you miss the Fibonacci number 2, which should be included in your sum.

Number 2:
I don't get the point of your for loops:
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
for (int i = 0; i < size; ++i){
	cout << "\n";
	if (nums[i] < MaxSize){
		cout << nums[i];
	}
	for (int i = 0; i < size; ++i){
		if (nums[i] < MaxSize){
			if ((i + 2) < size){
				total = total + nums[i + 2];
			}
			else{
				total = total + nums[i];
			}
			
			
		}
	}
}

I think this will help you: a % b will give you the remainder when a is divided by b, so for instance, 5 % 3 = 2 and 17 % 6 = 5.
Your for loop can be simplified considerably if you use that fact.
long double main: for some reason the last 2 or 3 numbers that get stored in my array are over 4million so the first for loop you were talking about checks if its under 4000000 and then tells me what they are. I originally did this just so i could check and make sure i was actually finding fibonacci numbers. The second one is supposed to check for even numbers, and by looking at the numbers I think that if I add two to i, I get to an even one. That is totally the wrong way to look for the even numbers and shouldn't be included in the post. I need to start commenting my code from now on..it really is confusing.
I think the code needs to be simplified. In the first loop, there is
nums.push_back() three times.
I would aim to generate just one number in the series in each pass through the loop, and just do push_back() one time.

Your code may well be generating some or all of the correct numbers, but to me it looks unnecessarily complicated.

When it comes to finding the sum, all you need is to check that the current number is divisible by 2. if it is, add it to the total. Yet what I see is two nested for loops and several if/else statements. Again, I think it could be much simpler, one loop and one if statement.

Thanks everyone ! Got it down to 6 lines with the correct answer.
Topic archived. No new replies allowed.