The declaration int letter = 0;
is perhaps too early in the program. letter
is only used inside the while
loop, so it should be inside of the loop.
Initializing int letter = 0;
is misleading. The initial value of letter
is overwritten by the std::cin >> letter;
statement, so the initial value of 0
is never used, and could be removed.
while (1) {
should be written as while (true) {
. Both amount to an infinite loop, but the latter reads better.
26
is a magic number, so you might want to turn into a named constant. Still, it is a pretty obvious constant, so I’d actually be ok with leaving it as is. However ...
64
is a magic number which absolutely should be turned into a named constant ... or eliminated altogether. What does 64
represent? The ASCII character'@'
? Still not obvious what it is doing, or why. How about using the expression 'A' + (letter - 1)
? That is much clearer and better!
Problem: If the user enters the number "hello"
, the std:cin
stream will go into a fail
state, and executing std::cin >> letter;
subsequent times will continue to fail, and "The English Alphabet ... Try again"
will be repeated forever. You should check for the stream going into the fail
state, clear the error, then discard the invalid input, before returning to reading the next integer.
Outputting "The letter that coresponds [sic] to that value is"
inside the while
loop makes it look like you can enter several values and get the results in the loop, when in reality you can only get one translation before the program exits. You should loop for the user’s input, and break out of the loop once the input is valid, and print the translation outside, after the loop. Something like:
int letter;
do {
if (std::cin >> letter) {
if (letter >= 1 && letter <= 26)
break;
} else {
std::cin.clear();
std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
}
std::cout << "Try again\n";
} while (true);
std::cout << "The letter that corresponds ...