# Anyone mind "proof reading" my code for me?

• 02-15-2003
carrja99
Anyone mind "proof reading" my code for me?
Okay okay! I know this is a pretty simple program and, quite frankly, there isn't much that can be done to improve it. Just wanted to see if there were any optimizations that could be done to it to improve it a bit (I really don't see any though).

It's odd how easy this problem turned out to be! I just finished learning stacks and linked lists, so at first I tried to use stacks to implement it!! As that got so complicated, I finally discovered the easy way out :D

Code:

```#include <iostream.h> #include <fstream> using namespace std; void TellMoves(int n, char origin, char destination, char spare, ofstream& write); int main()         {         int n;         char x,y,z, filename[16];         ofstream write;                 cout << "Enter the file name to output to: ";         cin >> filename;         write.open(filename);                 if (write.fail())                 {                 cout << "That is an invalid filename!"<<endl;                 }         cout << "The beginning Pole(A, B, or C?)";         cin >> x;         cout << "The ending Pole?";         cin >> y;                 cout << "How many discs?";         cin >> n;         if (((x == 'A')&&(y == 'B'))||((x == 'B')&&(y == 'A')))                 z = 'C';         else if (((x == 'C')&&(y == 'B'))||((x == 'B')&&(y == 'C')))                 z = 'A';         else if (((x == 'A')&&(y == 'C'))||((x == 'C')&&(y == 'A')))                 z = 'B';         TellMoves(n, x, y, z, write);                 return 0;         } void TellMoves(int n, char origin, char destination, char spare, ofstream& write) /* tells the user how to move the blocks to solve the puzzle; the n-story tower gets moved from 'origin' to 'destination' with 'spare' as the empty peg */         {           if (n == 1) // base case                   {                 cout << "Move disc " << n << " from Pole " <<origin << " to Pole "                <<destination <<endl;                                 write << "Move disc " << n << " from Pole " <<origin << " to Pole "                <<destination <<endl;                 }           else                   {                   // first, move the top n-1 blocks from the origin peg to the spare peg                   TellMoves(n-1, origin, spare, destination, write);                   // now move the bottom block from the origin peg to the destination peg                   cout << "Move disc " << n <<" from Pole " <<origin << " to Pole "                <<destination <<endl;                                 write << "Move disc " << n <<" from Pole " <<origin << " to Pole " <<destination <<endl;                   // finally, move those n-1 blocks back on top of the bottom block                   TellMoves(n-1, spare, destination, origin, write);                   }         }```
• 02-15-2003
ronin
use cin.getline(filename, sizeof(array), '\n') or similar so that you don't overflow it.

write.open(filename);

if (write.fail())
{
cout << "That is an invalid filename!"<<endl;
}

You've got a potential problem here. If the file does fail you do nothing other than print a message. Further down in your code you use write in your function.

Give a better error message and an attempt to correct the error should it occur or abort the program.

I'm not going to touch that if( || && ) else stuff as my head hurts just looking at it. :D
• 02-16-2003
Munkey01
Only include fstream, since the fstream header include the iostream header ;)
• 02-16-2003
beege31337
I just have a style recommendation:
use more descriptive variable names; but really only nessesarry if this was a larger, more complex program, and/or it was being written for a job.
• 02-16-2003
carrja99
Thanks for the pointers. I pretty much just whipped it up real quick before I left campus and just didn't have time to check any potentials errors in the code.

The if-else statements are solely to assign a value to the spare pole, i.e. if the Origin pole is A and the Destination pole is C, then z should be B. Of course, I realize my nonsensical variable names probably leave people clueless, which I plan to fix later.

Thanks!