12
\$\begingroup\$
#include <iostream>

int main() {
    int letter = 0;
    std::cout << "Please type in a number: ";

    while (1) {
        std::cin >> letter;
        //less than 1 or greater than 26
        if (!(letter < 1 || 26 < letter)) {
            std::cout << "The letter that corosponds to that value is '" 
                      << char(64+letter) << "'\n";
            return 0;
        }
        std::cout <<"The English Alphabet only has 26 letters. Try again\n";
    }
}

Gets a integer and prints it as a character. I had thought about using stdio instead of stream but I didn't want it to look scary to a beginner.

\$\endgroup\$

3 Answers 3

25
\$\begingroup\$

C++ does not mandate a particular character coding, so while your code is functionally correct on ASCII and ISO-8859 systems, it won't work in EBCDIC environments, for example. Even with the obvious fix to add 'A'-1 in place of the magic constant 64, the program assumes that English letters have consecutive values, which just isn't the case in BCD encodings.

The portable approach is to index a string literal:

    static const std::string_view alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";

    if (letter > 0 && letter <= alphabet.size()) {
        std::cout << "Letter " << letter << " is '"
                  << alphabet[letter-1]
                  << "'\n";
    }

Other issues:

  • Don't use letter before checking whether std::cin >> letter succeeded.
  • Instead of an infinite loop, prefer a finite loop for reading inputs.
  • Consider accepting input as command-line argument(s) as a more convenient alternative to using standard input.
  • Spelling: "corresponds".
\$\endgroup\$
1
  • 6
    \$\begingroup\$ I'm always amazed what you can learn from an at first glance trivial task. \$\endgroup\$ Mar 1, 2019 at 14:02
14
\$\begingroup\$

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 ...
\$\endgroup\$
5
  • \$\begingroup\$ You might want to check up on the correct usage of [sic]. In your answer you're using it to indicate that you changed the cited text to have correct spelling, while you should use [sic] to indicate that the original author made an error and you just cite as it was written. In case it's just autocorrect trolling you (and me), ignore what I just said. Or rather tell me and I'll delete my comment. ;) \$\endgroup\$
    – Inarion
    Mar 1, 2019 at 14:13
  • 1
    \$\begingroup\$ @Inarion Yup! Autocorrect fixed what I had left unfixed, and clearly I blinked and didn’t notice. I’ve uncorrected it to the proper (intended) usage. \$\endgroup\$
    – AJNeufeld
    Mar 1, 2019 at 15:41
  • 2
    \$\begingroup\$ Actually, @Ludisposed mistakenly corrected the spelling (presumably not seeing the [sic] there). \$\endgroup\$ Mar 1, 2019 at 15:44
  • \$\begingroup\$ Oops my bad! I am a simple man, I see red squiggle I do correct \$\endgroup\$
    – Ludisposed
    Mar 1, 2019 at 15:47
  • \$\begingroup\$ "corresponds" has two r's, doesn't it? \$\endgroup\$ Mar 3, 2019 at 0:58
7
\$\begingroup\$

I have five improvements to suggest. The first two are about sanitizing inputs, and are very important. The next two are about user interface design, and are minor. The last is about coding style, and is completely optional.

  1. Fail elegantly on non-integer inputs. This can be done using the fail(), clear(), and ignore() methods of std::cin.

  2. Respond to EOF appropriately. This can be done using std::cin.eof().

  3. Repeat the prompt after printing an error message.

  4. Check spelling of "corresponds". Do not capitalize "alphabet"

  5. In general, I dislike returning inside of a loop. For a clearer control flow, I would loop until getting a valid input, then, outside the loop convert to a letter.

#include <iostream>
#include <limits>

int main() {
    int letter = 0;

    while (1) {
        std::cout << "Please type in a number: ";
        std::cin >> letter;

        if (std::cin.eof()) {
            std::cout << std::endl;
            return 1;
        } else if (std::cin.fail()) {
            std::cin.clear();
            std::cin.ignore(std::numeric_limits<std::streamsize>::max(),'\n');
            std::cout <<"Integer input required. Try again.\n";
        } else if (letter < 1 || letter > 26) {
            std::cout <<"The English alphabet only has 26 letters. Try again.\n";
        } else {
            break;
        }
    }

    std::cout << "The letter that corresponds to that value is '" 
          << char(64+letter) << "'\n";

    return 0;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.