Search code examples
c++vectorindexingcollatz

How do I find the starting number from the sequence with the Max Value


In my code I computed the Collaz sequences of the range from start to end. I ran the code and everything prints correctly except for the starting element of the sequence with the greatest value. I debugged and saw that my max is correct however I end up printing the end for the starting number of the sequence with the greastest value. I tried resetting i and I also tried to see if there was an issue with my if statement

Ideally it should look like this once correct Example Program:

Start: 1

End: 6

Max Value: 16 (3)

Here is my code

int main()
{
  // This declare the int start and end and prints them to the user as well as takes what the user gives 
  int start, end;
  std::cout << "Start: ";
  std::cin >> start;
  std::cout << "End: ";
  std::cin >> end;

  // Here a vector called sequences is created. This holds a Collatz Sequences
  std::vector<CollatzSequence> sequences;
  int i = start;
  while (i <= end)
  {
    sequences.push_back(CollatzSequence(i));
    i = i + 1;
  }

  {
    // A vector is made to hold the collatz sequences
    std::vector<int> maxValues;
    i = 0;
    while (i < sequences.size())
    {
      maxValues.push_back(sequences[i].getMax());
      i = i + 1;
    }
    // Here this declares varibles as intagers
    int max = maxValues[0];
    int maxIndex = 0;
    {
      // The i (index) is declare as  integer
      int i = 1;
      // While i is less then the size of the vector maxValues
      while (i < maxValues.size())
      {
        // If true (the element at maxValue is greater then the max)
        if (maxValues[i] > max)
          // max value will be that element
          max = maxValues[i];
          // maxIndex will be the idex at maxValue or address
          maxIndex = maxValues[i];
        // i is then incremented which mean it will go through the sequence at each address
        i = i + 1;
      }
      std::cout << "Max Value: " << max << " (" << sequences[maxIndex].getStart() << ")\n";
    }
  }

And here is where I define my functions

CollatzSequence::CollatzSequence(int start)
 int CollatzSequence::getMax()
{
  int max = getNumbers()[0];
  {
    for (int i = 1; i < getNumbers()[i]; i++)
      if (getNumbers()[i] > max)
        max = getNumbers()[i];
  }
  return max;
}
int CollatzSequence::getStart()
{
  int start = getNumbers()[0];
  return start;
}


std::vector<int> const & CollatzSequence::getNumbers()
{
  return numbers;
}

Please let me know if you see and error I don't or if I did something wrong. I tried to only post what is needed and not post 4 files. Thank you for your time.


Solution

  • I guess you solved the problem by yourself right now. But anyway, lets answer the question so that it is not an 0-answer count.

    The bug is in that line:

    // maxIndex will be the idex at maxValue or address
    maxIndex = maxValues[i];
    

    You do not store the index, but the value. And that could be a big value and hence, out of bounds.

    The correct solution is:

    // maxIndex will be the idex at maxValue or address
    maxIndex = i;
    

    That should work.

    Just for the fun of it, I created a "more-modern-C++"-elements solution using the algorithm library. Please see:

    #include <iostream>
    #include <string>
    #include <vector>
    #include <algorithm>
    
    // Collatz Sequence and its max value
    struct CollatzSequence {
    
        CollatzSequence() {}
    
        // This will construct the collatez sequence and calculate the max value
        explicit CollatzSequence(unsigned int value); 
    
        // Values of the Collatz Sequence
        std::vector<unsigned int> data{};
    
        // The maximum value of all values of the Collatz sequence
        unsigned int maxValue{ 1U };
    };
    
    // Constructor for the Collatz Sequence. Will calculate all values of the sequence and the maxim value
    CollatzSequence::CollatzSequence(unsigned int value) {
    
        // We calculate values as long as we did not find 1, always the last vale 
        // (There is no mathematicla evidence that this is always true)
        while (value > 1U) {
    
            // Store the current calculated value
            data.push_back(value);
    
            // Check, if this is the biggest value so far, and, if so, store it
            maxValue = std::max(value, maxValue);
    
            // Calculate next value in the row according to the Collatz Sequence rules
            value = ((value & 1U) == 0) ? (value >> 1U) : (3U * value + 1U);
        }
        // Always add 1 as the last element. This we do not need to calculate
        data.push_back(1U);
    }
    
    int main() {
    
        // Inform user to enter start end end value
        std::cout << "Calculate Collatz sequences. Please specify a range, a start and a end value:\n";
    
        // Read the start value and check, if this worked
        if (unsigned int rangeStart{}; std::cin >> rangeStart) {
    
            // Now read end value and check, if a correct range has been specified
            if (unsigned int rangeEnd{}; (std::cin >> rangeEnd) && rangeStart <= rangeEnd) {
    
                // Create a vector for Collatz Sequences. Allocate sufficent elements for the specified range
                std::vector<CollatzSequence> sequences(rangeEnd - rangeStart + 1);
    
                // Create all requested sequences
                std::generate(sequences.begin(), sequences.end(), [i = rangeStart]()mutable { return CollatzSequence(i++); });
    
                // Get the max element 
                std::vector<CollatzSequence>::iterator mve = max_element(sequences.begin(), sequences.end(),
                    [](const CollatzSequence& cs1, const CollatzSequence& cs2) { return cs1.maxValue < cs2.maxValue; });
    
                // Show result to the world
                std::cout << "Max Value: " << mve->maxValue << " (" << mve->data[0] << ")\n";
            }
            else {
                std::cerr << "\n*** Error: Invalid range specified\n";
            }
        }
        else {
            std::cerr << "\n*** Error: Could not get start value for range\n ";
        }
        return 0;
    }