Anyone mind "proof reading" my code for me?

This is a discussion on Anyone mind "proof reading" my code for me? within the C++ Programming forums, part of the General Programming Boards category; Okay okay! I know this is a pretty simple program and, quite frankly, there isn't much that can be done ...

  1. #1
    Registered User carrja99's Avatar
    Join Date
    Oct 2002
    Posts
    56

    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);
      		}
    	}
    I am Error. When all else fails, use fire.

    My Current Screenshot

  2. #2
    Casual Visitor
    Join Date
    Oct 2001
    Posts
    350
    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.
    Last edited by ronin; 02-15-2003 at 09:33 PM.

  3. #3
    _ Munkey01's Avatar
    Join Date
    Dec 2002
    Posts
    280
    Only include fstream, since the fstream header include the iostream header

  4. #4
    booyakasha
    Join Date
    Nov 2002
    Posts
    208
    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.

  5. #5
    Registered User carrja99's Avatar
    Join Date
    Oct 2002
    Posts
    56
    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!

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 23
    Last Post: 04-20-2009, 07:35 AM
  2. Extended ASCII Characters in an RTF Control
    By JustMax in forum C Programming
    Replies: 18
    Last Post: 04-03-2009, 08:20 PM
  3. Enforcing Machine Code Restrictions?
    By SMurf in forum Tech Board
    Replies: 21
    Last Post: 03-30-2009, 07:34 AM
  4. Values changing without reason?
    By subtled in forum C Programming
    Replies: 2
    Last Post: 04-19-2007, 10:20 AM
  5. Replies: 4
    Last Post: 01-15-2002, 11:04 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21