EEEEEeee I made a program!
Tell me of any bad habits I'm forming before I get used to them! (Other than not commenting shit)
#include <iostream>
#include "Collatz_function.h"
using namespace std;
int main()
{
int collatznumber;
int collatzednumber;
int numberofsteps;
bool debug;
short int debuginput;
numberofsteps = 0;
cout << "What number do you want to perform the Collatz conjecture on? ";
cin >> collatznumber;
cout << "Debug? (1/0) ";
cin >> debuginput;
if (debuginput==1) debug=true; // Have a feeling that this is kind of a dumb method to input a boolean.
collatzednumber = collatznumber;
while (collatzednumber > 1)
{
collatzcheck (collatzednumber);
if (numberiseven) collatzednumber /= 2;
else collatzednumber = (collatzednumber * 3) + 1;
if (debug) cout << " ";
if (debug) cout << collatzednumber;
numberofsteps++;
}
return (numberofsteps);
}
#ifndef COLLATZ_FUNCTIONS
#define COLLATZ_FUNCTIONS
using namespace std;
bool numberiseven = true;
int collatzcheck (int a)
{
if ((a%2)==0) numberiseven = true;
else numberiseven = false;
}
#endif
On top of what kai pointed out, I have a few more suggestions.
cout << "Debug? (1/0) ";
cin >> debuginput;
if (debuginput==1) debug=true; // Have a feeling that this is kind of a dumb method to input a boolean.
I can't think of a better way to say this, so here goes: Your comment is correct. Slightly better:
cout << "Debug? (1/0) ";
cin >> debuginput;
debug = debuginput; // Yay casting! The only downside to this is that any non-zero integer value will make debug true. Considering that the previous code allowed any non-zero integer to make debug false, that's still a slight improvement.
Slightly better-er:
cout << "Debug? (true/false) ";
cin >> boolalpha >> debug;
The boolalpha flag allows booleans to be read and written as "true" and "false" in streams, rather than 1 and 0. This method allows you to get rid of the debuginput variable, which means you saved at least 2 bytes of memory. Yay! In addition, "short int" is almost never used. Use "short" instead. The "int" is implied.
Why do you have both collatznumber and collatzednumber? collatznumber is read in from standard input, has its value assigned to collatzednumber, and then is never used again. You can combine those two variables into one.
You are returning numberofsteps from the main method. That's bad practice. The return value from the main method is used to indicate how the program exited. 0 means everything was successful, and non-zero values indicate various errors, defined by the program and/or operating system. Write numberofsteps to standard output, and return 0 instead.