Here are some things that may help you improve your code.
Eliminate unused variables
The program currently doesn't do anything with
argv, so you could simply use
int main() instead. This would give a strong clue to the reader of the code that there are no command line options.
Use all required
The code uses
exit(1) but doesn't include the corresponding header. The code should have
And then use
std::exit(1). Alternatively, since it's already in
main, you could just use
return 0 instead of
Use string concatenation
The menu includes these lines:
cout << "Menu Lab v1" << endl;
cout << "--------------" << endl;
cout << endl;
cout << "A) Display Names in list" << endl;
cout << "Enter your choice:";
Each of those is a separate call to
operator<< but they don't need to be. Another way to write that would be like this:
cout << "Menu Lab v1\n"
"A) Display Names in list\n"
"Enter your choice:";
This reduces the entire menu to a single call to
operator<< because consecutive strings in C++ (and in C, for that matter) are automatically concatenated into a single string by the compiler.
Don't duplicate important constants
The filename is hardcoded right now (see next suggestion), but worse than that, it's done in two completely indpendent places. Better would be to create a constant:
static const char *FILENAME = "namelist.txt";
Consider the user
Instead of having a hardcoded filename, it might be nice to allow the user to control the name and location of the file. For this, it would make sense to use a command line argument and then pass the filename to the functions as needed.
Be consistent with file operations
addName the file is explicitly close, but in
openFile it is not. This is because in
openfile is a local variable that gets destroyed when the function returns, so the file is automatically closed. Either is acceptable, but it would be a good idea to just do it one way consistently. Similarly, the code uses the idiomatic combination declaration and open for
openFile but uses two steps in
addName. The latter could be rewritten as:
ofstream outfile(FILENAME, std::ios_base::app);
Separate user I/O from program functions
addName function, there are really two things happening: getting the name from the user and then appending that name to the file. Instead of interleaving those operations, I'd recommend separating them into separate functions. This would allow for better error reporting if any of the operations fail (e.g. if the file can't be opened). One possibility:
void appendName(const char *filename, string &name)
ofstream outfile(filename, std::ios_base::app);
outfile << name << endl;
cout << "Please type a name you would like to add:";
cin >> name;
cout << "Name added!" << endl;
Add error handling
File operations can fail, so your program should both detect and handle such failures. The one place the program currently checks a file operation at the moment is, unfortunately, not actually useful because it has no effect. Specifically this:
cout << getcontent << endl;
operates just like this:
cout << getcontent << endl;
Consider improving names
addName is not a bad function name, but
openFile is, since that function does more than simply open a file, and particularly because it also contains a variable named
openfile (which is also a bad name in my opinion). Perhaps
listNames would be a better name.