Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I want to print a nice loading message with these three fading dots, while the main thread does some heavy IO stuff. This is why I implemented this:

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

static bool            loadingStop;
static pthread_mutex_t loadingMutex;

void *loadingDots(void *message);

int main() {
  loadingStop = false;

  // MUTEX
  pthread_mutex_init(&loadingMutex, NULL);

  // ATTRIBUTE
  pthread_attr_t attr;
  pthread_attr_init(&attr);
  pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE);

  // THREAD
  pthread_t loadingThread;
  pthread_create(&loadingThread, &attr, loadingDots, (void *)"Loading (main, does something long)");

  pthread_attr_destroy(&attr);

  // SOMETHING TIME CONSUMING 
  usleep(8000000);

  // PLEASE 'LoadingDots' TO STOP
  pthread_mutex_lock(&loadingMutex);
  loadingStop = true;
  pthread_mutex_unlock(&loadingMutex);

  // WAIT FOR IT TO FINISH
  pthread_join(loadingThread, NULL);
  pthread_mutex_destroy(&loadingMutex);

  printf("This is printed after 'LoadingDots' savely quit.\n");

  return 0;
}

void *loadingDots(void *message)
{
  int c = 0;

  while (1) {
    switch (c) {
    case 0:
      printf("\r%s   \r%s", (char *)message, (char*)message);
      break;
    default:
      putchar('.');
      break;
    }

    fflush(stdout);
    usleep(1000000);

    c = (c+1) % 4;

    pthread_mutex_lock(&loadingMutex);
    if (loadingStop) { break; };
    pthread_mutex_unlock(&loadingMutex);
  }

  putchar('\n');
  pthread_exit(NULL);
}
share|improve this question

Mostly minor stuff

  1. Good use of fflush(stdout); to insure output. Something often missed.

  2. Minor: int main() { is not explicitly allowed per the standard. Better to use int main(void) {

  3. Minor: Naked physical quantity. Values like height, weight, time, should have units made clear feet vs meter $327.6 million mistake

    // usleep(8000000);
    #define SOME_us_TIME_CONSUMPTION (8000000 /* us */)
    usleep(SOME_us_TIME_CONSUMPTION);
    
  4. Minor: Avoid printf() when fputs() will do. Avoid % issue.. BTW did you mean "safely" vs "savely"?

    // printf("This is printed after 'LoadingDots' savely quit.\n");
    fputs("This is printed after 'LoadingDots' savely quit.\n", stdout);
    // or 
    puts("This is printed after 'LoadingDots' savely quit.");
    
  5. Long lines: Respect the width for the target audience - in this case code review.

    // pthread_create(&loadingThread, &attr, loadingDots, (void *)"Loading (main, does something long)");
    
  6. vs.

    pthread_create(&loadingThread, &attr, loadingDots, 
        (void *)"Loading (main, does something long)");
    
  7. Consider auto format. The space between bool loadingStop will likely shrink with auto formatting. Insure code is presentable even after auto formatting.

    // looks OK
    static bool loadingStop;
    static pthread_mutex_t loadingMutex;
    
  8. Minor: Not a fan of {} one liners

    // if (loadingStop) { break; };
    if (loadingStop) { 
      break;
    };
    
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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