Please comment on my program (Simple TXT Writer)

[bo0st99]

*Alright the scoping problem has been fixed, it was actually easier to fix than I thought. Anyway it's fixed now.

/*
 * Simple TXT Writer, by Danny Knight
 * Saturday, September 6, 2008
 * 
 * This program was made as practice with
 * many basic and standard C++ concepts
 * and libraries.  I commented everything
 * in great detail so that others can 
 * learn from it.
 *
 * Notes --------------------------------
 * -when writing to a file, after you have
 *  entered a line, you cannot go back and
 *  edit the line
 * -cannot enter a file name greater than
 *  one word long
 * -after adding to a text file, the new
 *  and old data are seperated by a blank
 *  line (I did not do this on purpose, I
 *  just couldn't figure out a quick and
 *  easy way to fix it)
 */

#include <iostream> // Standard C++ input / output stream
#include <string> // C++ string type
#include <fstream> // Standard C++ file stream

using namespace std; // Using standard namespace for standard C++ libraries

static void create_file(void); // Is run when the 'Create TXT' option is selected
static void open_file(void); // Is run when the 'Open TXT' option is selected 
void cannot_open_file(char* title_of_file); // Displays an error message including the name of the file as an ARRAY OF CHARs to the standard output stream and exits the program returning '1'
void cannot_open_file(string title_of_file); // Displays an error message including the name of the file as a STRING to the standard output stream and exits the program returning '1'
void cannot_open_file(); // Displays an error message to the standard output stream and exits the program returning '1'

void main()
{
	char option = ' '; // char being used to store the user's menu option

	while (!(option == 'e' || option == 'E'))
	{
		cout << "Welcome to Simple Text Reader" << endl;
		cout << "=============================" << endl;
		cout << "A - Create TXT\nB - Open TXT\nE - Exit" << endl;

		// Checks for valid input 
		for (;;)
		{
			cin >> option; // get value of option from keyboard
	
			if (option == 'A' || option == 'a') // if option a is selected, leave a line, run option a's function and break the forever loop
			{
				create_file();
				break;
			} 
			else if (option == 'B' || option == 'b') // if option b is selected run option b's function and break the forever loop
			{
				open_file();
				break;
			} else if (option == 'E' || option == 'e')
				exit(0);
			else cout << endl <<"please enter an 'a' or a 'b' or an 'e'" << endl; // Otherwise display error message and continue looping until the user had entered valid input
		}
		system("cls"); // Clear the screen before the Main Menu shows
	}
}	
	
static void create_file() // Option a's function
{
	string file_name; // Name of the file
	string info; // Contents of the file
	bool has_ext = false; // Weather or not the user put a .txt at the end of 'file_name'
	ofstream output_file; // Object of the output file
	
	system("cls"); // Clear the output window

	cout << "Name your file: "; // prompt the user to name the file, initilizing 'file_name'
	cin >> file_name;

	for (int i = 0; i < file_name.length();i++) // run the loop up to as many times as there are letters in 'file_name'
	{
		if ((file_name[i] == '.') && (file_name[++i] == 't' || file_name[++i] == 'T') &&
			(file_name[++i] == 'x' || file_name[++i] == 'X') && 
			(file_name[++i] == 't' || file_name[++i] == 'T') && file_name[++i] == '\0') // if the user did put a .txt at the end of 'file_name', set has_ext to true and break the loop
		{ has_ext = true; break; } else; // Otherwise keep looping until the end of 'file_name', (if there is no .txt, has_ext remains as false, its initial value)
	}

	// if has_ext is false, meaning the user did not put a .txt at the end of 'file_name', put .TXT at the end of it for them
	if (has_ext == false)
		file_name = file_name + ".TXT"; else; // Otherwise do nothing and continue with the program

	output_file.open(file_name.c_str()); // Create the output file by calling the 'open' function on the 'ofstream' object, and pass in an object of string, casted to a primative C string using the function c_str because open doens't take C++ 'string's
	if (!(output_file.is_open())) // If the file is unable to open, run the 'cannot_open_file' function
		cannot_open_file(file_name); else; // Otherwise do nothing and continue with the program
	
	// The following two lines are written to the text document created
	output_file << "This document is created in Simple Text Reader" << endl;
	output_file << "==============================================" << endl;

	cout << "Writing to text file " << file_name << " (type $save to save)" << endl; // Prompt the user to write to the file, entering "$save" to save their work
	while(!(info == "$save")) // Loop until "$save" is entered
	{
		getline(cin,info); // Gets a line of text from the 'cin' function and reads it into the 'info' object
		if (info != "$save") // Only write 'info' to file if it is not "$save" (this condition is another way of writing the condition in the while loop)
			output_file << info << endl;
		
	}

	output_file.close(); // Close output file
}

static void open_file() // option b's function
{
	string file_name; // Name of the file
	string info[30]; // Contents of the file
	string new_info; // A string that contains the appended contents of a file if the user selects 'Edit'
	bool has_ext = false; // Weather or not the user put a .txt at the end of 'file_name'
	ifstream input_file; // Object of the input file
	ofstream add_to_file; // This objcet opens a file as output, so that we can add the old data as well as the new stuff
	char option; // Add to or Man Menu
    int x; // Declared up here rather than in the for loop so that it can be used outside the for loop's block

	system("cls"); // Clear the output window

	cout << "Enter the name of the file to open" << endl;
	cin >> file_name;
	for (int i = 0; i < file_name.length();i++) // run the loop up to as many times as there are letters in 'file_name'
	{
		if ((file_name[i] == '.') && (file_name[++i] == 't' || file_name[++i] == 'T') &&
			(file_name[++i] == 'x' || file_name[++i] == 'X') && 
			(file_name[++i] == 't' || file_name[++i] == 'T') && file_name[++i] == '\0') // if the user did put a .txt at the end of 'file_name', set has_ext to true and break the loop
		{ has_ext = true; break; } else; // Otherwise keep looping until the end of 'file_name', (if there is no .txt, has_ext remains as false, its initial value)
	}
		// if has_ext is false, meaning the user did not put a .txt at the end of 'file_name', put .TXT at the end of it for them
	if (has_ext == false)
		file_name = file_name + ".TXT"; else; // Otherwise do nothing and continue with the program

	input_file.open(file_name.c_str());
	if (!(input_file.is_open())) // If the file is unable to open, run the 'cannot_open_file' function
		cannot_open_file(file_name); 
	else cout << "\nViewing " << file_name << "...\n" << endl; // Otherwise tell the user what file they are viewing
		
	for (x = 0;!(input_file.eof());x++) // Function 'eof' returns true when the file is out of data (eof stands for "end of file")
	{
		getline(input_file,info[x]);
		cout << info[x] << endl;
	}
	
	input_file.close();

	cout << "===================" << endl;
	cout << "A - Add to this TXT\nB - Main Menu" << endl;
	cout << "===================" << endl;
	
	for (;;)
	{
		cin >> option;
		switch(option)
		{
			case 'b':
			case 'B':
				return; // Ends function, returning to main
			case 'a':
			case 'A':
				{
					system("cls");
					add_to_file.open(file_name.c_str());
					if (!(add_to_file.is_open()))
						{ cout << "Unable to open file for output" << endl; exit(1); }

					for (int c = 0;c < x;c++) // The for loop incrementor is how the language got it's name, because it is an "incrementation" of the language C..
					{
						cout << info[c] << endl;
						add_to_file << info[c] << endl;
					}

					while (new_info != "$save") // Another way of writing (!(new_info == "$save"))
					{
						getline(cin,new_info);
						if (!(new_info == "$save"))
							add_to_file << new_info << endl; else;
					}
					return;
				}
		}
	}
}

void cannot_open_file(char* name) // See prototype
{
	cout << "Unable to open file: " << name << endl;
	exit(1);
}

void cannot_open_file(string name) // See prototype
{
	cout << "Unable to open file: " << name << endl;
	exit(1);
}

void cannot_open_file() // See prototype
{
	cout << "Unable to open file" << endl;
	exit(1);
}

[bo0st99]

I made this program so that people can tell me what I could have done better and how. Since this program was written to be viewed, I commented the crap out of it. The idea is that I get feedback that I, and others, can learn from.

P.S - I put a list of bugs that I didn't work out in the top comments, if someone could please post how to fix them, that would be cool.

[bo0st99]

P.P.S - Feel free to copy / paste the code into your own IDE to test it. It should run, I only used standard C++ libraries. I wrote this in Microsoft Visual C++ v6.0 (the old one)

[bo0st99]

Note: I just tested this program in Dev-C++, it gives me an compiler error regarding scoping because, if you notice, I used a variable declared in a for loop, outside the loop's block. When I was doing this, I thought that I would get an error for sure, but I tried it anyway and it worked. Maybe it's a visual studio thing or something...

-I'm not going to fix it because it's late and im tired but if you are interested in the code and you use Dev-C++, try another IDE, such as Visual C++ Express Edition, it's free, just google it.

[helios]

Don't double-post. It is frowned upon. The same applies to quadruple posting, but with more intensity. There's an edit function for a reason.

The same error will occur in any compiler post-2000. VC++ 6.0 follows only part of the standard. If you care at all about the quality of your code, you'll fix it so at least gcc can compile it (practically a requirement for any C++ program that doesn't use any special libraries).

Those aren't bugs. They are issues.
You can't write a proper text editor using streams. You need to use some console library like ncurses.
The second item comes from your misunderstanding of the behavior of std::cin.
The third one is probably a logic error easily fixed.

General style: Prefer the style

//Comment
statement;

over the style

statement; //Comment 

This makes the code easier to edit.

And on that same line, avoid this:

//Comment
statement;
//Comment
statement;
//Comment
statement;
//Comment
statement;

Instead, do this:

/*
Comment.
Comment.
Comment.
Comment.
Comment.
*/
statement;
statement;
statement;
statement;
statement;

Needless to say, hyperverbose comments is not a good thing.

1. main should return an int, not void.
2. Line 40: The condition will always be false. You examine the condition inside the loop at line 60 and finish if it's true.
3. Lines 42-44: Condense these into a single string.
4. Line 62: Whenever possible (i.e. always), put if and else statements in lines of their own to increase readability.
5. Line 64: Just don't.
6. Line 75: Again.
7. Line 78: std::cin will stop reading at the first whitespace it finds. This is the cause of issue 2. You can use getline, as you have on other places.
8. Lines 80-84: Read this: http://www.cplusplus.com/reference/string/string/find.html
9. Line 85: else statements aren't mandatory. If you have nothing to do in them, just leave them out.
10. Line 89: Instead of setting a flag, why not just add the extension in the previous if?
11. Lines 97 and 98: See comment 3.
12. open_file(): A significant portion of this function is duplicate code. You might want to move it to main().
13. Loop starting on line 142: What happens if the file has more that 30 lines? I'll tell you what happens. A fence post error happens, that's what.
14. Lines 152-154: See comment 3.
15. Line 166: Use the std::ios::app flag to open the file. Also, comment line 173. This somewhat fixes issue 3.
16. Line 168: Just awful.
17. Line 170: Here is the scope error. Also, completely irrelevant comment.
18. Line 176: Don't do more string comparisons than are necessary. They aren't cheap. Replace the condition with a 1 and put a break in the else block.
19. cannot_open_file(): All of its overloads are essencially the same function. You can do the same with a single function using templates.

That's all for now.

[Umz]

WOW!!! Thats some impressive feedback! Well done helios you've got hawk eyes, nothing gets past you.
Yea but about line 15, you can have a double named file to open, it works with VC++, might not work with Dev tho. I've never had a problem with it myself.

[bo0st99]

helios, thanks for the feedback, that is exactly what i was hoping for when i posted this. sorry about the double post thing, its just that when i was posting it said i only get a max of 8192 characters, and my code alone took up 8500something so i dont even know how that posted, but i didnt want to over do it and end up getting into trouble or something so i spread my posts out, but ur right even though, i could have put everything into 2 posts rather than 4. About my comments, i posted this not knowing what the prefered style was, so thank you i learned something =D. but i choose to do "statment; // comment" because when i am telling someone how many lines my code is, it is more accurate, yeh i know, dumb reason. I know that if statments can exist without and else statment, but i belieave that every if has an else, every open has a close, every malloc has a free, ect, ect. Thats why i put empty else's. I noticed about half way though that i was making my text editor the proper way, but i decided that id do it anyways, just to learn from. Why not use system("cls");? what would be a good alternative? Yeh i know that main should return int, a 0, to show that the program exited without errors, i use void because i actually dont know how to view the return statment of my program after it runs (check if it was 0 or 1 to determine if it had error or not). I understand cin and getline, but the thing is i tried using getline to read in the name of the file, and it returns an empty string. why does it do this? anyways, thats why i used cin and complained that i wasnt getting more than 1 word. i printed out your reply, i actually learned a lot from you, im new to C++ and it really helps thanks again. Iv heard of templates and looked into them, but i could never actually figure out how to use them. I saw the tutorial on this site, it doesnt really help to much, i think ill find some at school to teach me or something. And as for that scope error, i think i should fix it because it would make sence for it not to work in most IDE compilers and it makes sence to me for it not to work.

[helios]

Why not use system("cls");

There's a thread practically dedicated to the evils of system() and to interfaces: http://www.cplusplus.com/forum/beginner/1988/
If he sees this, Duoas will probably be better at explaining it than me.
The best alternative is to not clear the screen or to use a console library.

I understand cin and getline, but the thing is i tried using getline to read in the name of the file, and it returns an empty string. why does it do this?

You're carrying over a '\n' from the last std::cin you used. Put std::cin.ignore(1,'\n') before using readline().

i think i should fix it because it would make sence for it not to work in most IDE compilers and it makes sence to me for it not to work.

Huh?