1

I'm trying to solving Project Euler Problem 14. It asks to find the number under 1 million that generates the longest sequence. What I did was create a vector, v, and populate its elements with the length of the sequence for a particular number. Thus, the element that resides in position 13 will correspond to the length of the sequence generated by the number 13, and so on. However, some seemingly random elements take very large numbers and I can't figure out what's wrong with the code. Also, when I test it with 1,000,000, I get a completely wrong answer, but I know the program is working for some small numbers after testing them by hand and verifying.

#include <iostream>
#include <vector>
using namespace std;

void find_longest(int n)
{
    int count = 1;
    int max = 0;
    int position;

    vector<int> v;
    v.push_back(0);
    v.push_back(0);

    for(int i = 1; i < n; i++)
    {
        long long int trainer = i;
        count = 1;
        while(trainer != 1)
        {
            if(trainer%2 == 0)
            {
                trainer /= 2;
                count++;
            }
            else
            {
                trainer = 3*trainer + 1;
                count++;
            }
        }
        v.push_back(count);
    }

    vector<int>::iterator it;

    for(it = v.begin(); it < v.end(); it++)
    {
        cout << v[*it] << endl;
        //if(v[*it] > max)
        //{
        //    max = v[*it];
        //    position = *it;
        //}
    }

    //cout << "The longest sequence is " << max << " terms long and is ";
    //cout << "generated by the number " << position << "." << endl;
}

int main()
{
    find_longest(100);
    //find_longest(1000000);
}
Cœur
  • 37,241
  • 25
  • 195
  • 267
Bob John
  • 3,688
  • 14
  • 43
  • 57
  • I'm not seeing where you store the overall maximum total. I see "max" at the top of your code, but I never see "count" overwriting "max" if "count" > "max". – Inisheer Aug 18 '12 at 18:43
  • isn't `*it` the number? Not `v[*it]`? – John Corbett Aug 18 '12 at 18:44
  • 1
    Also, I'm confused why you're storing anything within a vector<>. I believe you should just store "max" and "value" for the starting number that created the longest sequence. – Inisheer Aug 18 '12 at 18:44
  • I commented out the part where I find max within the vector using the iterator; I believe it works correctly, but I may be wrong. @JohnCorbett, the number that generates a particular sequence is *it, correct. The actual sequence length would be v[*it], wouldn't it? – Bob John Aug 18 '12 at 18:46
  • 1
    No, the argument to operator[] is an index, not an iterator. What you need is `max = *it`, and the `position` will be distance between `v.begin()` and current iterator, e.g. `position = std::distance(v.begin(), it);` What `v[*it]` does is index into vector with value that iterator refers to. It's wrong logic. – jrok Aug 18 '12 at 18:48
  • That said, you don't need a vector, really. Just store a running max inside your first for loop. – jrok Aug 18 '12 at 18:51
  • I get the correct answer when I add a simple if-statement to the for-loop to find the max/position. But I wanted to get more creative and use a vector for reasons I didn't specify. I think I understand now @jrok, thanks for clarifying. – Bob John Aug 18 '12 at 18:53
  • 1
    You're welcome. Oh, and there's a neat algorithm in standard library to find a maximum value in a range: `max = std::max_element(v.begin(), v.end());` It's in `` header. – jrok Aug 18 '12 at 18:54
  • Cool! I'll check it out. – Bob John Aug 18 '12 at 19:20

1 Answers1

0

//removing change for type mismatch

You don't need to remember all N numbers in a vector. All you need is current sequence length. Then you calculate sequence length for the next number and if it is bigger than what you have already, you just keep the biggest one.

artapet
  • 171
  • 4