Use C++ headers instead of C headers. Change #include <stdlib.h>
to #include <cstdlib>
. You could actually remove this header altogether because you really should not be calling exit()
in this program.
Avoid using namespace std;
. You are polluting the global namespace and increasing the probability of naming clashes. If you put using namespace std;
into a header, you pollute the global namespace of every user that uses your header. Putting it into a CPP file is not as bad as putting it into a header, but the same dangers exist. See this link for more information.
Avoid using global variables when they are not necessary. You want to limit the scope of your variables as much as possible. This program can be rewritten to use no global variable at all.
Use appropriate function names. mainmenu()
should be either main_menu()
or MainMenu()
, writetofile()
should be either write_to_file()
or WriteToFile()
, etc.
Get rid of the unnecessary recursion. Recursion is very nice an clean in some situations. Unfortunately, it's very ugly in this situation. This program can be rewritten to use no recursion at all.
Learn about RAII. It is one of the most important aspects of the C++ language. In your writetofile
function, you manually call writefile.close();
. You do not have to do this. The std::ofstream
will automatically clean up its own resources once it goes out of scope. This is one of the reasons why limiting the scope of your variables is important.
Learn about std::numeric_limits. While cin.ignore(10000,'\n');
should work in most cases, std::cin.ignore(std::numeric_limits <std::streamsize>::max(), '\n');
will work in more cases.
Now with that out of the way, let's see how we can rewrite this program.
Headers
#include <fstream>
#include <iostream>
#include <limits> // for std::numeric_limits
#include <string>
Create a function to get user input
We want to get rid of the ugly recursion that we have going on. One thing we can do is create a helper function that can read user input for various values. This is a function template. It can return int
s, std::string
s, and other variable types. If bad input is given to this function, then it asks the user to try again.
template <typename T>
T get_input(const std::string &strQuery)
{
std::cout << strQuery << "\n> ";
T out = T();
while (!(std::cin >> out)) {
std::cin.clear();
std::cin.ignore(std::numeric_limits <std::streamsize>::max(), '\n');
std::cout << "Error!" "\n";
std::cout << strQuery << "\n> ";
}
return out;
}
Rewrite the mainmenu()
function
First we will rename mainmenu()
to main_menu()
. Second we will use our new get_input()
function. Notice that in C++, two string literals next to each other (such as "[1] Login" "\n") are automatically concatenated into one string literal by the compiler. Even if the string literals are on separate lines, as long as there are no semicolons or anything else between them, then the compiler will concatenate them. Using this strategy make your main_menu()
class easier to read. Notice that we only need two switch cases. All other cases result in nothing being done.
void main_menu()
{
int choice = get_input <int>(
"Hello, Would you like to log in or register?" "\n"
"[1] Login" "\n"
"[2] Register" "\n"
"[3] Exit");
switch (choice)
{
case 1:
login();
break;
case 2:
register_user();
break;
}
}
Rewrite the login()
function
This is a very small change. I add a newline to it.
void login()
{
std::cout << "You are being logged in!" "\n";
}
Rewrite the registerme()
function: Part 1
First, let's give it a better name. Let's rename this function to register_user()
. This function does 3 things. It:
- Gets a username.
- Gets a password.
- Saves the password to a file.
To make this function more readable, we will create 3 helper functions to perform each of these tasks.
Get the username
We will create a helper function that will get the username. We will use get_input()
to avoid all of the ugly recursion that you have.
std::string get_username()
{
std::string username = get_input <std::string>("Please enter a username.");
std::cout << "Username: \"" << username << "\"\n";
while (get_input <int>("Confirm? [0|1]") != 1) {
username = get_input <std::string>("Please enter a username.");
std::cout << "Username: \"" << username << "\"\n";
}
return username;
}
Get the password
We just have to make sure the passwords match. Once again, the get_input()
function makes this clean.
std::string get_password()
{
std::string password1 = get_input <std::string> ("Please enter your password.");
std::string password2 = get_input <std::string> ("Please re-enter your password.");
while (password1 != password2) {
std::cout << "Error! Passwords do not match." "\n";
password1 = get_input <std::string>("Please enter your password.");
password2 = get_input <std::string>("Please re-enter your password.");
}
return password1;
}
Save the username and password
Now we need a function that can save the username and password. This one is pretty similar to your writetofile()
function, except it is easier to read. It does require passing both the username and the password though.
void save_user(const std::string &username, const std::string &password)
{
std::string filename = username + ".txt";
std::ofstream file(filename);
file << password << "\n";
}
Rewrite the registerme()
function: Part 2
Now with those 3 helper functions implemented, we can write the new registerme()
function (which we renamed register_user()
). This turns your complicated function into a simple function that is easy to read.
void register_user()
{
std::string username = get_username();
std::string password = get_password();
save_user(username, password);
}
Rewrite the main()
function
Your main()
function stays pretty much the same. Just call main_menu()
instead of mainmenu()
since we renamed the function.
All of it together
#include <fstream>
#include <iostream>
#include <limits>
#include <string>
template <typename T>
T get_input(const std::string &strQuery);
std::string get_username();
std::string get_password();
void save_user(const std::string &username, const std::string &password);
void login();
void register_user();
void main_menu();
int main()
{
main_menu();
}
template <typename T>
T get_input(const std::string &strQuery)
{
std::cout << strQuery << "\n> ";
T out = T();
while (!(std::cin >> out)) {
std::cin.clear();
std::cin.ignore(std::numeric_limits <std::streamsize>::max(), '\n');
std::cout << "Error!" "\n";
std::cout << strQuery << "\n> ";
}
return out;
}
std::string get_password()
{
std::string password1 = get_input <std::string> ("Please enter your password.");
std::string password2 = get_input <std::string> ("Please re-enter your password.");
while (password1 != password2) {
std::cout << "Error! Passwords do not match." "\n";
password1 = get_input <std::string>("Please enter your password.");
password2 = get_input <std::string>("Please re-enter your password.");
}
return password1;
}
std::string get_username()
{
std::string username = get_input <std::string>("Please enter a username.");
std::cout << "Username: \"" << username << "\"\n";
while (get_input <int>("Confirm? [0|1]") != 1) {
username = get_input <std::string>("Please enter a username.");
std::cout << "Username: \"" << username << "\"\n";
}
return username;
}
void login()
{
std::cout << "You are being logged in!" "\n";
}
void main_menu()
{
int choice = get_input <int>(
"Hello, Would you like to log in or register?" "\n"
"[1] Login" "\n"
"[2] Register" "\n"
"[3] Exit");
switch (choice)
{
case 1:
login();
break;
case 2:
register_user();
break;
}
}
void register_user()
{
std::string username = get_username();
std::string password = get_password();
save_user(username, password);
}
void save_user(const std::string &username, const std::string &password)
{
std::string filename = username + ".txt";
std::ofstream file(filename);
file << password << "\n";
}