Fhyujk

#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 againn";

    }

}

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.

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 corosponds [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 againn”;

} while (true);



std::cout << “The letter that corresponds ...

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;

}

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".