-5
\$\begingroup\$

gotoxy() allows you to place the blinking text wherever you want. I wasn't able to get it to behave the way I wanted, so you will notice I had to insert a bunch of spaces in the cout portion of the blink function. Obviously this could be refined.

blink() I put a "/r" to set the cursor back to the beginning of the line, and then after a brief pause, I put another cout function with blank spaces and a "/r". This gives the blinking effect.

If you take a look at main, you will see the insertion of the gotoxy() function and the while loop.

The while loop allows the program to detect a specified key input "behind the scenes" so to speak. This means that it won't show a cursor, the input from the keyboard, insert a new line, space, or require the user to press enter. It simply waits for a specific key to be pressed before continuing the program.

// i'm honestly not sure which of these are necessary for this particular      
snip of code but ignore for now because in my program it is all needed. You  
can figure it out yourself if you want :D //
#include <iostream>
#include <conio.h> 
#include <Windows.h>
#include <chrono> 
#include <thread> 
#include <atomic>
#include <future>
////////////////////////////////////////////////////////////////////////////

// set cursor x/y position //
void gotoxy(int column, int line)
{
    COORD coord;
    coord.X = column;
    coord.Y = line;
    SetConsoleCursorPosition(
        GetStdHandle(STD_OUTPUT_HANDLE),
        coord
        );
}
////////////////////////////////

// blinking start function //
void blink(unsigned int interval_msecs, std::atomic<bool>& keep_at_it)
{
    while (keep_at_it)
    {
        std::cout << "                                    (s) Start\r" <<     
std::flush;
    std::this_thread::sleep_for(std::chrono::milliseconds(interval_msecs));
    std::cout << "                                             \r" <<  
std::flush;
    std::this_thread::sleep_for(std::chrono::milliseconds(interval_msecs));
    }
}
///////////////////////////////

// MAINMAINMAIN //
int main()
{
    // wait for 's' to proceed //
    gotoxy(0, 18);

    std::atomic<bool> keep_blinking(true);

    auto future = std::async(std::launch::async, blink, 500,     
std::ref(keep_blinking));

    bool start = false;

    while (start == false)
    {
        char key = ' ';
        key = _getch();
        if (key == 's')
        {
            start = true;
        }
    }
    ///////////////////////////////
}
////////////////// 
\$\endgroup\$
7
\$\begingroup\$

I want to congratulate you for not going the using namespace std route. It's a mistake many beginners make because they don't know any better and you went for safety instead of laziness.

You say you're not sure which of your includes you actually need. These are the ones necessary to run:

#include <iostream>
#include <conio.h> 
#include <Windows.h>
#include <future>

However, it's often preferred to include explicitly. If you use threads, include . Even if you don't have to because it's already included by another include or because your compiler sorted it out for you. If you refactor your code and removing one include suddenly breaks functions you didn't expect to break, you'll know you didn't include explicit enough. It's also a good overview for the next guy reading your code about what features are used. Including the same include twice won't matter for your program since the compiler will notice the include guards and only include every include once.

The following function could use a better name:

void gotoxy(int column, int line)

It's a function setting something on a specific coordinate. But the name of the function doesn't indicate what (it isn't part of a class either) and it's a void so it won't return a location. Your function goes, but nobody knows what's going with it. The arguments indicate they only know something about a row and column (while you're setting x and y).

Perhaps a better alternative would be:

void cursor_to_xy(int x, int y)

Notice the underscores make your function easier to read.

The comment above the call to gotoxy is quite useless:

// wait for 's' to proceed //

No, we're going somewhere. No waiting involved. So what's that comment doing in that location?

Now we're at comments anyway, a bunch of /////////////////////////////// trailing after a brace or on their own line are seldom useful. If you use them, let them indicate blocks. In this case extra white lines are used as well. Like:

int foo = 42;

///////////////////////////////
//    NEW BLOCK              //
///////////////////////////////

int bar = 24;

In their current state, they're just distracting.

The following doesn't help either:

// MAINMAINMAIN //
int main()

For starters there is only one main, not three. And we don't need a comment to indicate it's main because it's on the next line. It's like putting up an awfully big sign to indicate where your store is while the name of the store is also above the door and you're already at the entrance.

Your other comments are doing pretty much the same thing. They point out the obvious. No point in keeping them around.

This checks whether start is false:

while (start == false)

false is not true. C++ has a not operator: !

So you could also write:

while (!start)

Which does exactly the same.

The way you print your messages uses a lot of spaces. It's not easy to see how many spaces you're using and why exactly that many. The following can help:

std::string(36, ' ')

Where 36 is the amount of spaces to print. Of-course you could extract this number to a const which would make your code easier to maintain. That said, there are many things you could extract into const and other variables for increased maintainability. Note we need to include <string> for this to work.

The following code has all the above implemented plus one inconsistency fix. I leave it as an exercise to the OP to find it. Inconsistencies in writing style can lead to confusing code in larger projects and should be avoided.

#include <iostream>
#include <conio.h> 
#include <Windows.h>
#include <chrono> 
#include <thread> 
#include <atomic>
#include <future>
#include <string>

void cursor_to_xy(int x, int y)
{
    COORD coord;
    coord.X = x;
    coord.Y = y;
    SetConsoleCursorPosition(
        GetStdHandle(STD_OUTPUT_HANDLE),
        coord
        );
}

void blink(unsigned int interval_msecs, std::atomic<bool>& keep_at_it)
{
    while (keep_at_it)
    {
        std::cout << std::string(36, ' ') << "(s) Start\r" <<
            std::flush;
        std::this_thread::sleep_for(std::chrono::milliseconds(interval_msecs));
        std::cout << std::string(36+9, ' ') << "\r" <<
            std::flush;
        std::this_thread::sleep_for(std::chrono::milliseconds(interval_msecs));
    }
}

int main()
{
    cursor_to_xy(0, 18);

    std::atomic<bool> keep_blinking(true);

    auto future = std::async(
        std::launch::async,
        blink,
        500,
        std::ref(keep_blinking)
        );

    bool start = false;

    while (!start)
    {
        char key = ' ';
        key = _getch();
        if (key == 's')
        {
            start = true;
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Following the rule of making dependencies explicit, none of the include files in the OP are superfluous. Items from <chrono>, <thread> and <atomic> are being used. \$\endgroup\$ – glampert Nov 20 '15 at 17:07
  • \$\begingroup\$ @glampert Agreed. However, OP specifically asked which ones he needed and I wanted to answer that. Since there are usually include guards in the files included, it won't matter in the end. I should clear that up in my answer. \$\endgroup\$ – Mast Nov 20 '15 at 17:11
  • \$\begingroup\$ @glampert I've added an explanation and modified the code in light of your comment. \$\endgroup\$ – Mast Nov 20 '15 at 17:49
  • 1
    \$\begingroup\$ Great! It is a good review nevertheless. \$\endgroup\$ – glampert Nov 20 '15 at 18:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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