Removing Global Variables

[franz123]

Hello, I am fairly new to coding and I've been trying to find ways on how to get rid of a global variable in this code since I've learned it is not a good practice. I've been trying to get rid of it but the functions make it difficult. Are there other ways to set a default variable to an array and be able to change it? Any help would be appreciated.

#include <iostream>
#include <string.h>
using namespace std;

char default_username[64] = "default";
char default_password[64] = "admin";

bool credentials_are_equal(char input_username[], char input_password[]);
void input_credentials_prompt ();
void change_credentials_prompt();

int main()
{
	while (true){
		system("CLS");
		input_credentials_prompt();
		change_credentials_prompt();
	}	
}

bool credentials_are_equal(char input_username[], char input_password[]){
	if ((strcmp(input_username, default_username) == 0)
	&& strcmp(input_password, default_password) == 0){
		return true;	
		}
	return false;
}

void input_credentials_prompt () {
	char input_username[64], input_password[64]; 
    
    for (int number_of_attempts = 0; number_of_attempts < 3; number_of_attempts++){
	    cout << "Username: ";
	    cin >> input_username;
	    
	    cout << "Password: ";
	    cin >> input_password;
	    
	    if (credentials_are_equal(input_username, input_password)){
	    	system("CLS");
	    	cout << "Welcome to my world" << endl << endl;
	    	system("pause"); 
	    	system("CLS");
	    	return;
		} 
		system("CLS");
		if (number_of_attempts < 2)
			cout << "Wrong credentials, please login again." << endl;
		else {
			cout << "You've exceeded the allowable number of tries! Try again later!";
			exit(1);	
		}
	}
	
	
}

void change_credentials_prompt() {
	char option;
	cout << "Change Username and Password [Y/N]: ";
	cin >> option;
	system("CLS");
	
	if (tolower(option) == 'y'){
		char new_username[64], new_password[64];
		cout << "Input new username: ";
		cin >> new_username;
		
		cout << "Input new password: ";
		cin >> new_password;
		
		if (credentials_are_equal(new_username, new_password)){
			system("CLS");
			cout << "User's input matches the previous credentials. Are you sure you want to change the credentials? " << endl;
			change_credentials_prompt();
		}
		else {
			strcpy(default_username, new_username);
			strcpy(default_password, new_password);
			return;
		}
	}
	else if (tolower(option) == 'n'){
		cout << "Exiting program..";
		exit(1);
	}
	else {
	    cout << "Wrong input. Please try again. "<<endl;
		system("CLS");
		change_credentials_prompt();
	}
}

[Handy Andy]

Hello franz123,

First you should use the C++ "cstring" not the C "string.h".

Second it would be better to use a "std::string" than the C array.

For what they are make the variables constant

constexpr int MAXSIZE{ 64};

const char default_username[MAXSIZE = "default"

If "constexpr" is a problem just use "const".

As constant variables these can not be changed by the program, so using them as global variables is not a problem.

The problem with global is with a regular variable that can be changed and trying to find where it was given a wrong value when it should not have changed.

Andy

[dutch]

They should probably also just be pointers since there's no need for them to be arrays.

const char* const default_username = "default";

[oggin]

You could use byref functions.
Using C++ string to keep things simple, and there are a few lines to change, so it your own code back tweaked.

#include <iostream>
#include <string>
using namespace std;



string& default_username()  //byref
{
	static string d="default";
	return d;
}

string& default_password()  //byref
{
	static string d="admin";
	return d;
}

bool credentials_are_equal(string input_username, string input_password);
void input_credentials_prompt ();
void change_credentials_prompt();

int main()
{
	while (true){
		system("CLS");
		input_credentials_prompt();
		change_credentials_prompt();
	}	
}

bool credentials_are_equal(string input_username, string input_password){
			if (input_username== default_username()
				&& input_password== default_password()){
		return true;	
		}
	return false;
}

void input_credentials_prompt () {
	char input_username[64], input_password[64]; 
    
    for (int number_of_attempts = 0; number_of_attempts < 3; number_of_attempts++){
	    cout << "Username: ";
	    cin >> input_username;
	    
	    cout << "Password: ";
	    cin >> input_password;
	    
	    if (credentials_are_equal(input_username, input_password)){
	    	system("CLS");
	    	cout << "Welcome to my world" << endl << endl;
	    	system("pause"); 
	    	system("CLS");
	    	return;
		} 
		system("CLS");
		if (number_of_attempts < 2)
			cout << "Wrong credentials, please login again." << endl;
		else {
			cout << "You've exceeded the allowable number of tries! Try again later!";
			exit(1);	
		}
	}
	
	
}

void change_credentials_prompt() {
	char option;
	cout << "Change Username and Password [Y/N]: ";
	cin >> option;
	system("CLS");
	
	if (tolower(option) == 'y'){
		string new_username, new_password;
		cout << "Input new username: ";
		cin >> new_username;
		
		cout << "Input new password: ";
		cin >> new_password;
		
		if (credentials_are_equal(new_username, new_password)){
			system("CLS");
			cout << "User's input matches the previous credentials. Are you sure you want to change the credentials? " << endl;
			change_credentials_prompt();
		}
		else {
			
			default_username()= new_username;
			default_password()= new_password;
			return;
		}
	}
	else if (tolower(option) == 'n'){
		cout << "Exiting program..";
		exit(1);
	}
	else {
	    cout << "Wrong input. Please try again. "<<endl;
		system("CLS");
		change_credentials_prompt();
	}
} 

[seeplus]

pass std::string by const ref to avoid unnecessary copy if passed by value.

bool credentials_are_equal(const string& input_username, const string& input_password){
    return  input_username == default_username() && input_password == default_password();
}

[jonnin]

if the code is working and done, going back to clean it up may be very hard. The key is to not use them at all when writing new code once you realize they are generally bad practice.

As far as it goes, you can wrap global variables in a large established project into a class, using static class member variables, and they will still be effectively global and behave as before, but now you must explicitly call upon them so you can't accidentally have a local/global/parameter of the same name or otherwise think you are doing anything other than using a global variable. It puts it in a namespace of sorts and protects it to an extent, though the other problems with globals remain. It is generally better not to use this workaround but it may be the right answer for repairing ancient code.

if you want to fully replace them in a large project as a cleanup effort, this can be very difficult. You can use various techniques: declare them in main and pass them down to whoever needs them, for example, works but it can be a giant amount of edits to functions that need an additional parameter now. If that is overwhelming, the class wrapper idea may be useful, or you can try to bundle the functions that need it into a class that has it as a member...
Similar to the class wrapper, you can also use a function wrapper. Here again static is the key, eg

int &global()
{//this function wraps up a variable that used to be global to the code base.  
   static int glob;
   return glob;
}
then wherever you need it, you can say
int &g = global();

This is sort of the 'letter of the law' problem. You can do it, its terrible, but it solves the 'get rid of the globals' arbitrary code cleanup project with the least amount of time wasted. If you want a better answer, be prepared to do a rewrite or major surgery.

[deleted account xyzzy]

An alternative to global variables is have an anonymous namespace with private members:

https://www.internalpointers.com/post/c-namespaces-private-members

[mbozzi]

Unless I'm misreading the article, the guy suggests this program won't compile:

namespace A { namespace { int i; } }

int main()
{
    ++A::i; // accesses A::UNIQUE::i
}

but there's nothing wrong with it.

[z05DSL3A]

 In function 'int main()':
5:7: error: 'foo' has not been declared
 At global scope:
1:31: warning: 'A::{anonymous}::i' defined but not used [-Wunused-variable]

🤷‍♂️

[mbozzi]

@Grey Wolf: s/foo/A, sorry.

Ah, second time through it makes more sense, although it's still fairly disappointing.

The article can be summed up as
"If you don't need a name to be visible everywhere, don't declare it in a public header."

Namespaces have nothing to do with it.

[z05DSL3A]

I knew what you were getting at, it just amused me a bit.

[seeplus]

For one way to remove the global variables (which aren't const), consider:

#include <iostream>
#include <string>
#include <cctype>

struct Cred {
	std::string username;
	std::string password;

	Cred() {}
	Cred(const std::string& user, const std::string& pass) : username(user), password(pass) {}
};

bool credentials_are_equal(const Cred& curc, const Cred& newc);
void input_credentials_prompt(const Cred& curc);
void change_credentials_prompt(Cred& curc);

int main()
{
	Cred curc("default", "admin");

	while (true) {
		input_credentials_prompt(curc);
		change_credentials_prompt(curc);
	}
}

bool credentials_are_equal(const Cred& curc, const Cred& newc) {
	return newc.username == curc.username && newc.password == curc.password;
}

void input_credentials_prompt(const Cred& curc) {
	Cred newc;

	for (int number_of_attempts = 0; number_of_attempts < 3; ++number_of_attempts) {
		std::cout << "Username: ";
		std::cin >> newc.username;

		std::cout << "Password: ";
		std::cin >> newc.password;

		if (credentials_are_equal(curc, newc)) {
			std::cout << "Welcome to my world\n\n";
			return;
		}

		if (number_of_attempts < 2)
			std::cout << "Wrong credentials, please login again.\n";
	}

	std::cout << "You've exceeded the allowable number of tries! Try again later!\n";
	exit(1);
}

void change_credentials_prompt(Cred& curc) {
	char option {};

	do {
		std::cout << "Change Username and Password [Y/N]: ";
		std::cin >> option;

		if ((option = (char)std::tolower(option)) == 'y') {
			Cred newc;

			std::cout << "Input new username: ";
			std::cin >> newc.username;

			std::cout << "Input new password: ";
			std::cin >> newc.password;

			if (credentials_are_equal(curc, newc))
				std::cout << "User's input matches the previous credentials\n";
			else {
				curc = newc;
				return;
			}
		} else
			if (option == 'n') {
				std::cout << "Exiting program..\n";
				exit(1);
			} else
				std::cout << "Wrong input. Please try again.\n";
	} while (true);
}