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();
}
}
\b
for each character you wrote?std::string
has a nicesize()
function... Also, you should consider sleeping before deleting, now you print and delete immediately, then pause, so you can't really see what was printed. \$\endgroup\$