27
\$\begingroup\$

I've recently wanted to make a "Loading..." display in C where the dots print one at a time in order and then reset:

enter image description here

Suprisingly, there isn't much on the internet for doing this well, so I figured I would make a simple program for it.

#include <stdio.h>
#include <time.h>

int main(void)
{
    int msec = 0;
    const int trigger = 500; // ms
    const int printWidth = 4;
    int counter = 0;
    clock_t before = clock();

    while (1)
    {
        fputs("Loading", stdout);
        clock_t difference = clock() - before;
        msec = difference * 1000 / CLOCKS_PER_SEC;
        if (msec >= trigger)
        {
            counter++;
            msec = 0;
            before = clock();
        }
        for (int i = 0; i < counter; ++i)
        {            
            fputc('.', stdout);
        }
        for (int i = 0; i < printWidth - counter; ++i)
        {
            fputc(' ', stdout);
        }
        fputc('\r', stdout);
        fflush(stdout);

        if (counter == printWidth)
        {
            counter = 0;
        }
    }
}

The output given is seen in the .gif above.

I know this can be done better. Any suggestions for improvement?

\$\endgroup\$
  • 10
    \$\begingroup\$ What's up with the moving green box? Feature? \$\endgroup\$ – Mathieu Guindon Aug 23 '16 at 16:42
  • 3
    \$\begingroup\$ @Mat'sMug An artifact from printing the carriage return rapidly. \$\endgroup\$ – syb0rg Aug 23 '16 at 16:44
  • \$\begingroup\$ On a second look, it's of course only because the four dots -state is overwritten instantly. And that's dealt with by not being so quick to rewrite which was already handled in an answer so nevermind. :) \$\endgroup\$ – ilkkachu Aug 24 '16 at 12:54
31
\$\begingroup\$

Too frenetic

I ran your program but it was very frenetic. It was constantly clearing the "Loading" prompt and reprinting it which resulted in a flickering effect. In addition, the cursor also moved around in a flickery manner (similar to the green box in the animated image).

To improve this, I would do two things:

  1. Don't constantly draw when nothing has changed. Instead, sleep until the next trigger time and then redraw. This also has the added benefit of not using 100% of your cpu. Presumably, you will have another thread running which is doing some loading work, and you don't want this thread to hog cpu time.

  2. You don't need to clear and redraw the whole prompt until you get to the last dot. Up until that point, you can just draw one dot at a time.

Rewrite

I rewrote your program with the above two fixes in mind:

#include <stdio.h>
#include <unistd.h>

int main(void)
{
    const int trigger = 500; // ms
    const int numDots = 4;
    const char prompt[] = "Loading";

    while (1) {
        // Return and clear with spaces, then return and print prompt.
        printf("\r%*s\r%s", sizeof(prompt) - 1 + numDots, "", prompt);
        fflush(stdout);

        // Print numDots number of dots, one every trigger milliseconds.
        for (int i = 0; i < numDots; i++) {
            usleep(trigger * 1000);
            fputc('.', stdout);
            fflush(stdout);
        }
    }
}

Rewrite 2

In response to the comment where @syb0rg indicated that the main thread should not sleep, here is what I would do in that case:

#include <stdio.h>
#include <time.h>

static void redrawPrompt(void);
static void doWork(void);

int main(void)
{
    const int trigger   = (CLOCKS_PER_SEC * 500) / 1000;  // 500 ms in clocks.
    clock_t   prevClock = clock() - trigger;

    while (1) {
        clock_t curClock = clock();

        if (curClock - prevClock >= trigger) {
            prevClock = curClock;
            redrawPrompt();
        }
        doWork();
    }
}

static void redrawPrompt(void)
{
    static int  numDots;
    const  int  maxDots = 4;
    const  char prompt[] = "Loading";

    // Return and clear with spaces, then return and print prompt.
    printf("\r%*s\r%s", sizeof(prompt) - 1 + maxDots, "", prompt);
    for (int i = 0; i < numDots; i++)
        fputc('.', stdout);
    fflush(stdout);
    if (++numDots > maxDots)
        numDots = 0;
}

static void doWork(void)
{
    // This function does loading work but returns at least every 500 ms.
}
\$\endgroup\$
  • 1
    \$\begingroup\$ This will suspend/hang the current thread though, which is why I wrote a timer to handle this in the first place. Can you think of a better way to do it without sleeping? Also, unistd.h isn't usable on Windows systems. \$\endgroup\$ – syb0rg Aug 23 '16 at 18:01
  • \$\begingroup\$ why sizeof(prompt) - 1 instead of strlen(prompt)? \$\endgroup\$ – machine_1 Aug 23 '16 at 18:27
  • 4
    \$\begingroup\$ @machine_1, because in this case it is an array type. Using sizeof on it will yield it's size during compile time, whereas strlen is runtime. Another reason why arrays are not pointers (at least during compile time). \$\endgroup\$ – Incomputable Aug 23 '16 at 18:30
  • \$\begingroup\$ @syb0rg Why wouldn't you want to suspend the thread? Is it supposed to call into a work function? If so, then you could just call the work function and read a timer. But you wouldn't reprint the prompt unless the time had passed the next trigger point. \$\endgroup\$ – JS1 Aug 23 '16 at 20:16
  • 2
    \$\begingroup\$ @syb0rg See my rewrite #2. Maybe that is more what you were thinking of? \$\endgroup\$ – JS1 Aug 23 '16 at 20:33
12
\$\begingroup\$

You can get rid of that flickering green box by disabling the cursor.

fputs("\e[?25l", stdout); /* hide the cursor */

If you want it back, you can re-enable it.

fputs("\e[?25h", stdout); /* show the cursor */
\$\endgroup\$
  • \$\begingroup\$ Only if your terminal supports ANSI escape codes, e.g., not Windows cmd.exe \$\endgroup\$ – cat Aug 24 '16 at 0:21
  • 2
    \$\begingroup\$ @cat I do assume output is to a supporting environment for the codes, yes. A check with isatty could also help if output is redirected to a file, otherwise the file text would be messy. \$\endgroup\$ – syb0rg Aug 24 '16 at 2:41
  • \$\begingroup\$ @cat For a few months the Windows 10 cmd.exe has supported ANSI escape codes, but you'd be hard-pressed to get it to compile without Visual Studio. (Even Pelles C had some down-time!) \$\endgroup\$ – wizzwizz4 Aug 24 '16 at 9:44
  • \$\begingroup\$ @wizzwizz4 Are you serious? That's the most absurd thing I've heard all week. Anyways, I still count that as "no support" because it's a recent build that doesn't get shipped to users, and you have to build it yourself with vomiting sounds MSVC++. \$\endgroup\$ – cat Aug 24 '16 at 13:48
  • \$\begingroup\$ @cat It was shipped to my computer when I installed the operating system. See en.wikipedia.org/wiki/ANSI_escape_code#Windows_and_DOS \$\endgroup\$ – wizzwizz4 Aug 24 '16 at 14:28
8
\$\begingroup\$

I would probably tie this into whatever code is actually loading something, and print a new dot (or use a /-\| spinner) every 100 kb or whatever number makes, instead of trying to use time. This would also add some extra feedback if it's stalled or running slowly.

For the ... animation, cycle between printing . three times and then \b\b\b \b\b\b (three backspaces, three spaces, three backspaces) once, and only print anything when you're changing phases.

#include <stdio.h>
#include <time.h>

const char *dot_str[] = {".", ".", ".", "\b\b\b   \b\b\b"};
#define countof(x) (sizeof(x)/sizeof((x)[0]))

static int next_state = 0;
void update_progress(void) {
    fputs(dot_str[next_state], stdout);
    next_state = (next_state + 1) % countof(dot_str);
    fflush(stdout);
}

static time_t last_time = 0;
void update_progress_if_time(void) {
    time_t now = time(NULL);
    if(now > last_time) {
        update_progress();
        last_time = now;
    }
}

void start_progress(const char *loading) {
    fputs(loading, stdout);
    next_state = 0;
    last_time = 0;
    fflush(stdout);
}

For this example, I would use update_progress as a callback if you can arrange it to be called after a sufficiently large fixed unit of work, otherwise update_progress_if_time to update approximately every second. Maybe it might make sense to move the variables into a structure and pass it to the callback, but you've only got one standard output anyway.

int main(void) {
    start_progress("Loading");
    for(;;) {
        update_progress_if_time();
    }
}
\$\endgroup\$
  • \$\begingroup\$ I didn't want to constrain reviews too much in my question, but how I intend to use this code is to replace my "Listening: " cluttered output in one of my projects. This would be a good indication to the user that the system is still working without cluttering the output. \$\endgroup\$ – syb0rg Aug 24 '16 at 4:46
  • 1
    \$\begingroup\$ @syb0rg I added some example code. For your specific case you linked you might also want to "start" the animation whenever the first "talking" frame is received, and finish (erase it? print a newline? print a carriage return and a different message?) when the first silence frame is received. And instead of using the time-based version I'd simply pick an appropriate number of frames and call update_progress every Nth frame. \$\endgroup\$ – Random832 Aug 24 '16 at 4:55
  • \$\begingroup\$ What are the three spaces (between backspaces) for? \$\endgroup\$ – Amani Kilumanga Aug 25 '16 at 7:42
  • 2
    \$\begingroup\$ @AmaniKilumanga To erase the dots - the backspaces just move the cursor. \$\endgroup\$ – Random832 Aug 25 '16 at 16:27
  • \$\begingroup\$ @Random832 ah yes of course. \$\endgroup\$ – Amani Kilumanga Aug 25 '16 at 23:05
7
\$\begingroup\$

Instead of the double for loop: one for dots and one for spaces, you could write combined one:

for (int dotPosition = 0; dotPosition < printWidth; ++dotPosition)
{            
    fputc(dotPosition < counter ? '.' : ' ', stdout);
}

To wrap the counter around to 0 when hitting printWidth, you can use the modulo operator.

counter = (counter+1) % printWidth;

As said by others, there's no point in flushing all the time (it's a waste of water) Here's my version of your program.

#include <stdio.h>
#include <time.h>

int main(void)
{
    int msec = 0;
    const int trigger = 500; // ms
    const int printWidth = 4;
    int counter = 0;
    clock_t before = clock();

    while (1)
    {
        clock_t difference = clock() - before;
        msec = difference * 1000 / CLOCKS_PER_SEC;

        if (msec >= trigger)
        {
            // timer bookkeeping
            counter = (counter+1) % printWidth;
            msec = 0;
            before = clock();

            // string assembly
            fputs("Loading", stdout);
            for (int dotPosition = 0; dotPosition < printWidth; ++dotPosition)
            {            
                fputc(dotPosition < counter ? '.' : ' ', stdout);
            }

            // flush and rewind for next write
            fflush(stdout);
            fputc('\r', stdout);
        }
    }
}

I'm not exactly sure why, but it doesn't show the frenetic cursor teleportation either. It stays put at the end of the line.

\$\endgroup\$
  • 2
    \$\begingroup\$ The cursor doesn't teleport because here, the printing is slow enough for the terminal to keep up, but you still busyloop around the clock() call, fully using a cpu core. \$\endgroup\$ – ilkkachu Aug 24 '16 at 12:30
  • 1
    \$\begingroup\$ @ilkkachu thank you for the info. I thought the busy loop was intentional. I mostly use C for embedded stuff. It occurred to me that a timer interrupt would be the better way to do the interval, but I don't think there's a nice portable way to do something like this in vanilla C. I'd love to be corrected on that if I'm wrong. \$\endgroup\$ – I'll add comments tomorrow Aug 24 '16 at 12:40

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.