Thread: 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

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);
  		}
	}

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.