C++ console progress display

Question

I just figured out how to do a building menu with a progression feature in c++ console application.

What this code does:

It simply offers you a choice to build a building and then prints the progression like: 1% - 9% erasing and printing one number at a time (Which is the kind of the core feature that I struggled a lot with).

then prints "Building done".

I would like to have some thoughts and comments on how to:

1. Make this code simpler in any way .
2. Did someone else make the same feature and how did you implement it?
3. General thoughts on the code
4. How would you structure it?

---

std::string building;

std::cout << "-CONSTRUCTION MENU-" << std::endl << std::endl;

std::cout << "1. Barrack" << std::endl << std::endl;

std::cout << "Build: ";

std::cin >> building;

if (building == "1" || building == "Barrack" || building == "barrack")
{
    std::string sentence[10] = { "1%", "2%","3%", "4%", "5%", "6%", "7%", "8%", "9%" };

    for (int index = 0; sentence[index] != sentence[9]; index++)
    {
        std::cout << sentence[index] << std::flush;
        std::cout << "\b\b";
        Sleep(150);

        if (index == 8)
        {
            system("cls");
            std::cout << "Building done" << std::endl;
            system("pause");
            break;
        }
    }
}

Answer 1

There are several things you can do to improve this code.

First of all the code above isn't a valid C++ code. To run it you need to add:

#include <string>
#include <iostream>
#include <Windows.h>

int main()
{
    ... // your code posted above
}

Please next time you ask a question post the full code.

Then your Menu:

std::cout << "-CONSTRUCTION MENU-" << std::endl << std::endl;

std::cout << "1. Barrack" << std::endl << std::endl;

std::cout << "Build: ";

Consider writing it like this:

std::cout << "-CONSTRUCTION MENU-\n\n"
<< "1. Barrack\n\n"
<< "Build: ";

• \n should be used instead of std::endl. The reason is std::endl not only gives you a new line, it also does an expensive buffer flushing operation which is rarely necessary.

• No need to call std::cout several times after each other. Statements can be chained.

Then your Percentage display:

std::string sentence[10] = { "1%", "2%","3%", "4%", "5%", "6%", "7%", "8%", "9%" };

for (int index = 0; sentence[index] != sentence[9]; index++)
{
    ...// statements inside
}

Several things are wrong here:

• You are using a plain C array to hold the sentence variables. This is very bad because a C array does not know about its size. Instead you use the magic number '9' to finish iterating over the strings. Please study the default C++ containers std::array or std::vector. In this case I would go for array since you don't want to add more values at run time.
• The size is wrong. You declared space for 10 elements but only put in 9.
• sentence is a very bad name. You don't store sentences. Consider using percentages instead.

Now we can write this instead:

std::array<std::string, 9> percentages = { "1%", "2%","3%", "4%", "5%", "6%", "7%", "8%", "9%" };

for (const auto& sentence : sentences)
{
    ...// statements inside
}

So the world is OK now? Not at all. You shouldn't use an array for numbers in the first place. Consider building the numbers in a loop instead:

for (int i = 1; i <= 9; ++i) {
    std::cout << std::to_string(i) << '%' << "\b\b";
    Sleep(150);
}

I removed the std::flush here since I don't see its purpose.

As a side hint it is a good practice to use ++i instead of i++. If you use objects later instead of ints you save an expensive copy.

Now we come to another dark chapter of your snippet. Your program is mixed with non-portable statements:

Sleep(150);

Consider using std facilities here:

#include <chrono>
#include <thread>

...


std::this_thread::sleep_for(std::chrono::milliseconds(150));

The big advantage here is that this is portable now.

The next candidate is:

system("pause");

Since you just want the program to wait with a message at the end you can do this:

std::cout << "Press any key...";
std::cin.get();

The last statement we get to is this:

system("cls");

No easy answer here. The fact is that the standard does not provide a portable solution here. You could wrap the statement in a function and make it portable for different platforms like this:

void clear_screen()
{
#if defined _WIN32
    std::system("cls");
#elif defined __unix__
    std::system("clear");
#elif defined (__APPLE__)
    std::system("clear");
#endif
}

We should also replace the

#include <windows.h> 

with

#include <cstdlib>

So now also on Linux / MAC the clear should work.

As a last word. A function should only do one thing. You should separate the program in well defined parts. Here is my reworked solution with all the improvements already mentioned, dividing the program into parts:

#include <array>
#include <chrono>
#include <cstdlib>
#include <iostream>
#include <string>
#include <thread>

void clear_screen()
{
#if defined _WIN32
    std::system("cls");
#elif defined (__unix__) 
    std::system("clear");
#elif defined (__APPLE__)
    std::system("clear");
#endif
}

std::string get_user_choice()
{
    std::string choice;
    std::cout << "-CONSTRUCTION MENU-\n\n"
        << "1. Barrack\n\n"
        << "Build: ";
    std::cin >> choice;
    return choice;
}

void print_progress(int start, int end)
{
    for (auto i = start; i <= end; ++i) {
        std::cout << std::to_string(i) << '%' << "\b\b";
        std::this_thread::sleep_for(std::chrono::milliseconds(150));
    }
}

void print_building_completed()
{
    clear_screen();
    std::cout << "Building done\n";
    std::cout << "Press any key...";
    std::cin.get();
}

int main()
{
    auto building = get_user_choice();
    if (building == "1" || building == "Barrack" || building == "barrack"){
        print_progress(0, 100);
        print_building_completed();
    }
}