Collatz Conjecture (c++) I am new to C++ and this is my first program. The objective of this program is to fully complete the sequence called the Collatz conjecture.


  It concerns a sequence defined as follows: start with any positive integer n. Then each term is obtained from the previous term as follows: if the previous term is even, the next term is one half the previous term. Otherwise, the next term is 3 times the previous term plus 1. The conjecture is that no matter what value of n, the sequence will always reach 1.


I am not familiar with the language just yet and would appreciate some (constructive) feedback. The program works as intended, however, I have a feeling that some of my code is not as elegant as it could/should be.

#include &lt;iostream&gt;
#include &lt;ctime&gt;

using namespace std;

bool isValid(int num) {
  if(num &lt;= 0) {
    return false;
  } else {
    return true;
  }
}

bool isEven(int num) {
  if (num % 2 == 0) {
    return true;
  } else {
    return false;
  }
}

void calculate(int num) {
  if (num == 1) {
    cout &lt;&lt; endl &lt;&lt; "Finished!" &lt;&lt; endl;
    return;
  } else if(isEven(num)) {
    num = num / 2;
    cout &lt;&lt; num &lt;&lt; " ";
    calculate(num);
  } else {
    num = (num * 3) + 1;
    cout &lt;&lt; num &lt;&lt; " ";
    calculate(num);
  }
}

int main() {

  int input;
  cout &lt;&lt; "Please enter an integer greater than zero." &lt;&lt; endl;
  cin &gt;&gt; input;

  if(isValid(input)) {
    calculate(input);
  } else {
    cout &lt;&lt; "Not a valid input." &lt;&lt; endl;
    return 0;
  }

  return 0;
}
Good job checking the input for validity and breaking up the logic into simple functions! However, some improvements can be made.

Avoid using namespace std

This can cause name collisions because it adds every name in the std namespace to the global namespace. For a small program like this one it's unlikely that you'll run into any problems (then again, maybe not) but it's best to get into the habit of using the std:: prefix on names in the std namespace.

Alternatively, you can introduce using declarations like using std::cout; to add specific names to the global namespace.

Avoid std::endl in favor of \n

std::endl flushes the stream, which can cause a loss in performance. In some cases you can also reduce calls to operator&lt;&lt; (e.g. cout &lt;&lt; endl &lt;&lt; "Finished!" &lt;&lt; endl; becomes cout &lt;&lt; "\nFinished!\n";).

Simplify your boolean expressions

You can remove some of your explicit return true and return false statements by returning the expression in the if condition. For example, you can shorten isEven() like so:

bool isEven(int num) {
  return num % 2 == 0;
}


Similarly, you can shorten isValid().

Don't mix logic and writing to cout in calculate()

Currently calculate() is not very reusable since it writes to cout. What if you wanted to write to a different stream, or if you just wanted to calculate the next term in the sequence without writing to any stream? I suggest redefining calculate() to simply return the next term in the sequence:

int next(int prev) {
  if (isEven(prev))
    return prev / 2;
  else
    return prev * 3 + 1;
}


Then in main() you can output each number returned by next() to cout, as well as check for the condition in which the term equals 1. You would replace the calculate(input); statement in main() with something like this:

while (input != 1) {
  input = next(input);
  std::cout &lt;&lt; input &lt;&lt; " ";
}

std::cout &lt;&lt; "\nFinished!\n"