Print ASCII table

Question

Just starting, wrote simple program off the very basic knowledge I have learned today. Is this a correct way of implementing this? Any improvement tips?

//Print out the ASCII chart with its corresponding code
#include <iostream>
#include <string>

void printChart(char symbol, int code) {
    std::string ws = "White Space";
    std::cout << "CODE: " << code << " SYMBOL: ";
    if (isspace(code)) {
        std::cout << ws << "\n";
    }
    else
        std::cout << symbol << "\n";
}

int main() {

    char symbol = 0;
    int code = symbol;
    int char_range = 127;

    while (code < char_range) {
        if (isprint(code)) {
            printChart(symbol, code);
        }
        symbol += 1;
        code = symbol;
    }

    std::cout << "Strike any key to continue...";
    getchar();

    return 0;
}

Answer 1

Sure, it works, if the host-characterset extends ASCII, thus a good start. And naturally, we have loads of ideas.

1. printChart() receives the same input twice, using different types. Duplication is never good. Just pass it once and convert as needed inside.

2. printChart() always outputs to std::cout. That's fine for a one-off, but if you ever want to reuse it consider making the used output-stream an (optionally defaulted) argument (type std::ostream&).

3. Try to minimize the number of objects you create, especially those which might need dynamic allocation like std::string. You can just use a string-literal instead of ws, or at least change its type to const char*.

4. There's no reason not to handle char-values outside the unsigned char-range right if you get them as a char: Make sure to cast to unsigned char before passing to character-classification-functions like isspace and isprint.

5. If you want to select one of two expressions, consider using the conditional operator (exp ? true_exp : false_exp).

6. Try to keep to a consistent coding-style, it's a powerful tool for making code more readable:

   • Do single statements need their own block or not?
   • Don't add a new-line after opening a new block. There's a spurious one directly at the start of main().

7. Use a for-loop instead of emulating it with a while-loop.

8. You know you handle ordinals 0 to 126, while ASCII goes to 127? No matter, 127 is a control-character, "DEL".

9. You need to include the headers for isprint(), isspace(), and getchar(). Because you use them without std::-prefix, you need <stdio.h> and <ctype.h>, not <cstdio> and <cctype>. Still, using the latter and adding the prefix arguably leads to slightly more consistent code.

10. Input might be line-buffered, so "Strike any key to continue..." might actually wait for enter. Also, consider being consistent and using std::cin.get() instead.

11. return 0; is implicit for main().

Answer 2

It's usually advisable to avoid printing inside of simple functions. Functions are easiest to test and use when instead of doing something with their result, they return their results and let the caller do whatever they want with them. You have to print somewhere, but make sure the place you're printing from makes sense.

What if in the future you wanted to use your function to format a string into its codes, but don't want to print the codes? What if you want to actually use them?

Prefer returning results to carrying out side effects with them. If you want to print the results, return them from the function, and print them at the call site. For your code here, you may want to change your printing function to a function that just formats the character. Then, you can cout the return from the function. This still removes the "deciding" code from the main, which can help readability.

This might look something like:

std::string formatChar(char symbol, int code) {
    std::string ws = "White Space";

    // Create a string stream to assist concatenation
    std::stringstream ss;

    // Add strings and numbers to the stream. 
    ss << "CODE: " << code << " SYMBOL: ";

    // Decide what string to add, and add it. 
    if (isspace(code)) {
        ss << ws << "\n";

    } else {
        ss << symbol << "\n";
    } 

    // Turn the string stream into a String, and return it. 
    return ss.str();
}

Then, change your main loop to:

while (code < char_range) {
        if (isprint(code)) {
            std::cout << formatChar(symbol, code);
        }
        symbol += 1;
        code = symbol;
    }

The benefits won't be clear here, but this will make your life significantly easier once you begin working on larger projects. Unfortunately the need for the string stream here kind of convoluted the point, but it's still something that should at least be kept in mind.

Answer 3

The name of printChart() misled me for a while. Its purpose is to print a single line of the chart, so perhaps it should be called printLine() (or even createLine() or similar, if you accept Carcigenicate's suggestion). Good names make a huge difference in the comprehension of your code, but inventing them is one of the hardest problems in programming!

Inside the function, there's std::string ws that is constructed on every call, whether it's used or not, and always with the same contents. This is obviously wasteful. One way to reduce the overhead is to to make it static so that it's only initialised once - it's worth making it const as well so you can be sure its value never changes:

    static const std::string ws = "White Space";

Looking at how we use it (streaming it to output), there's no benefit to std::string over an ordinary (null-terminated, C-style) string:

    static const char *const ws = "White Space";

(The two appearances of const on different sides of the * indicate that this is a constant pointer to constant characters - you can't modify ws and you can't modify ws[i]).

Alternatively, as an array:

    static const char ws[] = "White Space";

We can also reduce its scope, as it's used only in a small part of the code:

    if (isspace(code)) {
        static const char ws[] = "White Space";
        std::cout << ws << "\n";
    } else {
        std::cout << symbol << "\n";
    }

At this point, it's probably clearer to inline the one and only use, and not give it a separate name:

    if (isspace(code)) {
        std::cout << "White Space\n";
    } else {
        std::cout << symbol << "\n";
    }

(I agree with most of the suggestions in other answers; no need to duplicate them here).

Answer 4

UPDATE: I took into suggestion some of the ideas and re wrote the program to be "simpler". I realized that the program task itself being very simple made the use of a user defined function unnecessary. Let me know what you think of the code and if there is anything I may have missed.

//*****************EDITED VERSION********************
//Print out the ASCII chart with its corisponding code
#include <iostream>
#include <cstdio>
#include <cctype>

int main() {

    char symbol = 0;
    int char_range = 128;

    for (int i = 0; i < char_range; i++, symbol++) {
        std::cout << "CODE " << (int)symbol;
        if (std::isspace(symbol)) {
            std::cout << " SYMBOL: WHITE SPACE\n";
        }
        else {
            if (std::iscntrl(symbol)) {
                std::cout << " SYMBOL: CONTROL\n";
            } 
            else {
                std::cout << " SYMBOL: " << symbol << "\n";
                }
            }
        }

    std::cout << "Strike Enter to continue...";
    std::cin.get();

    return 0;
}