C++

Forum

 
How can I avoid long if...else if chains

Alienguard (8)
Hello everyone,
I am sorry if this is a double post sorts.
I am working on a random key generator as a practice excercise and I have this function takes input from the user. The idea is allow the user to choose how the key is to be created.

For examples, the program will ask the user these questions.

Should the key include uppercase letters?
Should the key include lowercase letters?
Should the key include special characters?
Should the key include numbers?

So, if the user answers no to all these questions, the program will generate an error message because it will be unable to generate a key.

If the user says yes to uppercase letters and no to the rest, the program will generate the key only with uppercase letters (e.g: PITH WSAMK)

If the user says yes to lowercase letters and to special characters and no to the rest, then the key will use a combination of lowercase letters and special characters (e.g: p=*^#wet q@#tre!@])

As you already noted there are different combinations to be made which brings me to my actual situation. Having so many choices left me with this super massive if...else if statement checking for every possible combination. My question is, is there a way for me to avoid using such long if statements?

Thank you in advance.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
 
  if (uppercase
		&& !lowercase
		&& !specialChars
		&& !numbers)
	{
		//Create key out of uppercase letter only
	}
	else if (!uppercase
		&& lowercase
		&& !specialChars
		&& !numbers)
	{
		//Create key with lowercase letters only
	}
	else if (!uppercase
		&& !lowercase
		&& specialChars
		&& !numbers)
	{
		//Create key using special characters only
	}
	else if (!uppercase
		&& !lowercase
		&& !specialChars
		&& numbers)
	{
		//Create key using numbers only
	}
	else if (uppercase
		&& lowercase
		&& !specialChars
		&& !numbers)
	{
		//Create key out of uppercase and lower case letters only
	}
	else if (!uppercase
		&& lowercase
		&& specialChars
		&& !numbers)
	{
		//Create key out of lowercase letters and special characters
	}
	else if (!uppercase
		&& !lowercase
		&& specialChars
		&& numbers)
	{
		//Create key out of special characters and numbers
	}
	else if (uppercase
		&& !lowercase
		&& !specialChars
		&& numbers)
	{
		//Create key out of uppercase letters and numbers
	}
	else if (!uppercase
		&& lowercase
		&& !specialChars
		&& numbers)
	{
		//Create key out of lowercase letters and numbers
	}
	else if (uppercase
		&& !lowercase
		&& specialChars
		&& !numbers)
	{
		//Create key out of uppercase letters and special characters
	}
	else if (!uppercase
		&& lowercase
		&& specialChars
		&& numbers)
	{
		//Create key out of lowercase letters, special characters and numbers
	}
	else if (uppercase
		&& !lowercase
		&& specialChars
		&& numbers)
	{
		//Create key out of uppercase letters, special characters and numbers
	}
	else if (uppercase
		&& lowercase
		&& !specialChars
		&& numbers)
	{
		//Create key out of letters and numbers
	}
	else if (uppercase
		&& lowercase
		&& specialChars
		&& !numbers)
	{
		//Create key out of letters and special characters
	}
keskiverto (10402)
Perhaps some containers and iteration:
1
2
3
4
5
6
7
8
9
10
11
12
 
const std::array<std::string,4> sets {"abc", "DEF", "#%&", "890" };
std::array<bool,4> selection;
// update selection

std::string charset;
for ( size_t s=0; s < selection.size(); ++s ) {
  if ( selection[s] ) charset += sets[s];
}

if ( charset.empty() ) std::cout << "I'm sorry, Dave. I'm afraid I can't do that.\n";

// generate key from charset
Duthomhas (13212)
There is nothing wrong with if..else chains.

But you can often obviate them altogether.

Method 1
As keskiverto suggests, start with containers or sets of only those possible characters to select from.

Method 2
Randomly select from all characters until you get one that satisfies one of the selected character classes.

A simple example of the second method:
1
2
3
4
5
6
7
8
9
 
while (key.size() < desired_size)
{
  char c = new_random_value;
  if (!uppercase and std::isupper( c )) continue;
  if (!lowercase and std::islower( c )) continue;
  if (!numbers   and std::isdigit( c )) continue;
  ...
  key += c;
}
JLBorges (13770)
> My question is, is there a way for me to avoid using such long if statements?

In general, it is a good idea to avoid deep nesting of conditionals and loops.
Excessive straight-line function length and excessive block nesting depth ... are twin culprits that make functions more difficult to understand and maintain, and often needlessly so. Each level of nesting adds intellectual overhead when reading code because you need to maintain a mental stack (e.g., enter conditional, enter loop, enter try, enter conditional, ...). Have you ever found a closing brace in someone's code and wondered which of the many fors, whiles, or ifs it matched? Prefer better functional decomposition to help avoid forcing readers to keep as much context in mind at a time.

Exercise common sense and reasonableness: Limit the length and depth of your functions.

- Alexandrescu and Sutter in 'C++ Coding Standards: 101 Rules, Guidelines, and Best Practices'


Factoring the code into (small) functions make the code simpler. For example:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
 
#include <iostream>
#include <random>
#include <string>

// compose the alphabet from which keys are to be generated
std::string make_alphabet() ;

// generate a random key using characters from the given alphabet
std::string make_key( const std::string& alphabet, std::size_t min_length, std::size_t max_length ) ;

int main()
{
    const std::string alphabet = make_alphabet() ;

    if( alphabet.empty() ) std::cerr << "error: empty alphabet, unable to generate key\n" ;
    else std::cout << "generated key: " << make_key( alphabet, 12, 30 ) << '\n' ;
}

// ask the user if the character class is to be included, return true if yes
static bool want( const std::string& char_class_name )
{
    std::cout << "should the key include " << char_class_name << " (y/n)? " ;
    char answer ;
    std::cin >> answer ;
    return answer == 'Y' || answer == 'y' ;
}

std::string make_alphabet()
{
    static const std::string upper_case_letters = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" ;
    static const std::string lower_case_letters = "abcdefghijklmnopqrstuvwxyz" ;
    static const std::string numbers = "0123456789" ;
    static const std::string special_chars = "!@#$%^&*()_+-=;':\",./<>?~`" ;

    std::string alphabet ;
    if( want( "upper case letters" ) ) alphabet += upper_case_letters ;
    if( want( "lower case letters" ) ) alphabet += lower_case_letters ;
    if( want( "numbers" ) ) alphabet += numbers ;
    if( want( "special characters" ) ) alphabet += special_chars ;
    return alphabet ;
}

std::string make_key( const std::string& alphabet, std::size_t min_length, std::size_t max_length )
{
    static std::mt19937 rng( std::random_device{}() ) ;
    static std::uniform_int_distribution<std::size_t> distrib ;
    using param_type = typename std::uniform_int_distribution<std::size_t>::param_type ;

    std::string key ;
    const std::size_t key_length = distrib( rng, param_type{ min_length, max_length } ) ;

    distrib.param( param_type( 0, alphabet.size()-1 ) ) ;
    while( key.size() < key_length ) key += alphabet[ distrib(rng) ] ;

    return key ;
}
Alienguard (8)
These are interesting answers, I never considered making a container or dividing everything into smaller functions. See, the way I am generating the key is directly from the ASCII table, so I didn't it would be necessary for me to store each character class in a different container.

I am attaching all the code I have written so far (I should have done this from the start, sorry). I curious if maybe by giving you more perspective, some of the expressed views would change. I am making this exercise inspired by Stroustrup's Programming Principles and Practice always wonderful book, so some code may be familiar to you.

I also want to thank everyone for their input. I will definitely consider the changes proposed. Expect a second iteration of the code.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
 
#include <cctype>
#include <string>
#include <iostream>
#include <random>

/*
Generates a random string based on user's length specification
returns true if successful
*/
bool generate_key(int length, bool uppercase, bool lowercase, bool specialChars, bool numbers, std::string &keyStr)
{
	if (!uppercase
		&& !lowercase
		&& !specialChars
		&& !numbers)
	{
		//Nothing to do here, so we return.
		return false;
	}
	
	//Values to store the range of the ASCII table
	int lowerValue = std::numeric_limits<unsigned char>::min();
	int maxValue = std::numeric_limits<unsigned char>::max();

	//Random number generator to generate a character
	std::random_device rd;
	std::mt19937 valueGenerator(rd());
	std::uniform_int_distribution<> dist(lowerValue, maxValue);

	if (uppercase
		&& !lowercase
		&& !specialChars
		&& !numbers)
	{
		//Create key out of uppercase letter only
	}
	else if (!uppercase
		&& lowercase
		&& !specialChars
		&& !numbers)
	{
		//Create key with lowercase letters only
	}
	else if (!uppercase
		&& !lowercase
		&& specialChars
		&& !numbers)
	{
		//Create key using special characters only
	}
	else if (!uppercase
		&& !lowercase
		&& !specialChars
		&& numbers)
	{
		//Create key using numbers only
	}
	else if (uppercase
		&& lowercase
		&& !specialChars
		&& !numbers)
	{
		//Create key out of uppercase and lower case letters only
	}
	else if (!uppercase
		&& lowercase
		&& specialChars
		&& !numbers)
	{
		//Create key out of lowercase letters and special characters
	}
	else if (!uppercase
		&& !lowercase
		&& specialChars
		&& numbers)
	{
		//Create key out of special characters and numbers
	}
	else if (uppercase
		&& !lowercase
		&& !specialChars
		&& numbers)
	{
		//Create key out of uppercase letters and numbers
	}
	else if (!uppercase
		&& lowercase
		&& !specialChars
		&& numbers)
	{
		//Create key out of lowercase letters and numbers
	}
	else if (uppercase
		&& !lowercase
		&& specialChars
		&& !numbers)
	{
		//Create key out of uppercase letters and special characters
	}
	else if (!uppercase
		&& lowercase
		&& specialChars
		&& numbers)
	{
		//Create key out of lowercase letters, special characters and numbers
	}
	else if (uppercase
		&& !lowercase
		&& specialChars
		&& numbers)
	{
		//Create key out of uppercase letters, special characters and numbers
	}
	else if (uppercase
		&& lowercase
		&& !specialChars
		&& numbers)
	{
		//Create key out of letters and numbers
	}
	else if (uppercase
		&& lowercase
		&& specialChars
		&& !numbers)
	{
		//Create key out of letters and special characters
	}

	//Create key using all the available characters
	unsigned char codeValue = ' ';
	for (unsigned i = 0; i < length;)
	{
		codeValue = static_cast<char>(dist(valueGenerator));
		if (std::isprint(codeValue))
		{
			std::cout << "Pushed character: " << codeValue << '\n';
			keyStr.push_back(codeValue);
			i++;
		}
		continue;
	}
	std::cout << "Generated code is: " << keyStr << '\n';
	return true;
}

void skip_to_int()
{
	//We found something that wasn't an integer
	if (std::cin.fail())
	{
		std::cin.clear();	//We'd like to look at the characters
		//Throw away all non digits
		for (char ch; std::cin >> ch;)
		{
			if (std::isdigit(ch) || ch == '=')
			{
				std::cin.unget();
				return;
			}
		}
	}
	std::cout << "No input\n";
}

int get_int(const std::string& sorry)
{
	int n = 0;
	while (true)
	{
		if (std::cin >> n)
		{
			return n;
		}
		std::cout << sorry;
		skip_to_int();
	}
}

/*Takes ranged input and returns the a validated integer*/
int get_int(int low, int high, const std::string& greeting, const std::string& sorry)
{
	std::cout << greeting << ":[" << low << ':' << high << "]\n";

	while (true)
	{
		int n = get_int("Sorry, that was not a number, please try again\n");
		if (low <= n && n <= high)
		{
			return n;
		}
		std::cout << sorry << ":[" << low << ':' << high << "]\n";
	}
}

char validate_character(const std::string& sorry)
{
	char userChoice = ' ';
	while (true)
	{
		if (std::cin >> userChoice)
		{
			std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
			if (std::isalpha(userChoice))
			{
				if (std::isupper(userChoice))
				{
					std::tolower(userChoice);
					return userChoice;
				}
				return userChoice;
			}
		}
		std::cout << sorry;
	}
}

/*Returns a validated boolean*/
bool get_yes_or_no_answer(const std::string& greeting, const std::string& sorry)
{
	
	std::cout << greeting << '\n';
	while (true)
	{
		char userChoice = validate_character("You did not entered an alphabetical character.\n");
		if (userChoice == 'y')
		{
			return true;
		}
		else if (userChoice == 'n')
		{
			return false;
		}
		std::cout << sorry;
	}
}

bool can_generate_key()
{
	int strLenght = get_int(1, std::numeric_limits<int>::max(), "Enter key length within range", "You are out or range");
	bool includeUpper = get_yes_or_no_answer("Do you want to include uppercase letters?", "Invalid character input, please try again\n");
	bool includeLower = get_yes_or_no_answer("Do you want to include lowercase letters?", "Invalid character input, please try again\n");
	bool includeSpecial = get_yes_or_no_answer("Do you want to include special characters?", "Invalid character input, please try again\n");
	bool includeNumbers = get_yes_or_no_answer("Do you want to include numbers?", "Invalid character input, please try again\n");

	if (!includeUpper
		&& !includeLower
		&& !includeSpecial
		&& !includeNumbers)
	{
		std::cout << "Key can't be generated, at least one condition needs to be true\n";
		return false;
	}

	return false;//generate_key(strLenght, includeUpper, includeLower, includeSpecial, includeNumbers);
}

int main()
{
//Hardcoded values to test function
	std::string CDKey = " ";
	generate_key(16, true, true, true, true, CDKey);
	return 0;
}
Last edited on
Alienguard (8)
So, I went through a second iteration as promised and this is a preliminary result. So far it works and does what I need it to. I must admit I shamelessly stole code from @JLBorges
Also thank you for the explanation and for referring me to the best practices guide. I will definitely give it a throughout read in future projects.

Here is the code for the second iteration.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
 
#include <cctype>
#include <string>
#include <iostream>
#include <random>


void skip_to_int()
{
	//We found something that wasn't an integer
	if (std::cin.fail())
	{
		std::cin.clear();	//We'd like to look at the characters
		//Throw away all non digits
		for (char ch; std::cin >> ch;)
		{
			if (std::isdigit(ch) || ch == '=')
			{
				std::cin.unget();
				return;
			}
		}
	}
	std::cout << "No input\n";
}

int get_int(const std::string& sorry)
{
	int n = 0;
	while (true)
	{
		if (std::cin >> n)
		{
			return n;
		}
		std::cout << sorry;
		skip_to_int();
	}
}

/*Takes ranged input and returns the a validated integer*/
int get_int(int low, int high, const std::string& greeting, const std::string& sorry)
{
	std::cout << greeting << ":[" << low << ':' << high << "]\n";

	while (true)
	{
		int n = get_int("Sorry, that was not a number, please try again\n");
		if (low <= n && n <= high)
		{
			return n;
		}
		std::cout << sorry << ":[" << low << ':' << high << "]\n";
	}
}

char validate_character(const std::string& sorry)
{
	char userChoice = ' ';
	while (true)
	{
		if (std::cin >> userChoice)
		{
			std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
			if (std::isalpha(userChoice))
			{
				if (std::isupper(userChoice))
				{
					std::tolower(userChoice);
					return userChoice;
				}
				return userChoice;
			}
		}
		std::cout << sorry;
	}
}

/*Returns a validated boolean*/
bool want(const std::string& greeting, const std::string& sorry)
{
	std::cout << greeting << '\n';
	while (true)
	{
		char userChoice = validate_character("You did not enter an alphabetical character.\n");
		if (userChoice == 'y')
		{
			return true;
		}
		else if (userChoice == 'n')
		{
			return false;
		}
		std::cout << sorry;
	}
}

/*
Generates a random string based on user's length specification
returns true if successful
*/
std::string generate_key(std::string &alphabet, int length)
{
	//Random number generator to generate a character
	std::random_device rd;
	std::mt19937 valueGenerator(rd());
	std::uniform_int_distribution<> dist(0, alphabet.size()-1);

	//Create key using all the available characters
	std::string CDKey;
	int codeValue = ' ';
	for (unsigned i = 0; i < length;)
	{
		codeValue = (dist(valueGenerator));
		CDKey.push_back(alphabet.at(codeValue));
		i++;
		continue;
	}
	std::cout << "Generated code is: " << CDKey << '\n';
	return CDKey;
}

std::string make_alphabet()
{
	static const std::string upper_case_letters = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
	static const std::string lower_case_letters = "abcdefghijklmnopqrstuvwxyz";
	static const std::string numbers = "0123456789";
	static const std::string special_chars = "!@#$%^&*()_+-=;':\",./<>?~`";

	std::string alphabet;
	int strLenght = get_int(1, std::numeric_limits<int>::max(), "Enter key length within range", "You are out or range");
	if (want("Do you want to include uppercase letters?", "Invalid character input, please try again\n")) alphabet += upper_case_letters;
	if (want("Do you want to include lowercase letters?", "Invalid character input, please try again\n")) alphabet += lower_case_letters;
	if (want("Do you want to include special characters?", "Invalid character input, please try again\n")) alphabet += special_chars;
	if (want("Do you want to include numbers?", "Invalid character input, please try again\n")) alphabet += numbers;


	if (alphabet.empty())
	{
		std::cerr << "I am sorry Dave, I am afraid I can't do that\n";
		return alphabet;
	}
	return generate_key(alphabet, strLenght);
}

int main()
{
	std::string CDKey = make_alphabet();
	std::cout << CDKey << '\n';

	return 0;
}
Topic archived. No new replies allowed.