Is My Countdown Timer Code Good or Does it Need Work?

0
3
Asked By CleverKoala42 On

I'm starting to learn C and wanted to create a basic countdown timer that can be paused, stopped, and restarted. I put together a simple implementation and would love some feedback on whether it's decent or if there are improvements I could make. Here's the code I came up with:

**Header File (timer.h):**
```h
#pragma once
#ifndef TIMER_H
#define TIMER_H

#include

typedef struct TimerThread {
pthread_t thread;
pthread_mutex_t mutex;

void* (*post)(void*);
double total;
double remaining;
int paused;
int stopped;
} TimerThread;

void* StartTimerThread(void* tt);

void toggle_pause(TimerThread*tt);

void start(TimerThread*tt);
void stop(TimerThread* tt);

#endif // !TIMER_H
```

**Source File (timer.c):**
```c
#include "timer.h"
#include

void* StartTimerThread(void* args)
{
TimerThread* tt_args = args;
pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, NULL);
struct timespec time = { 0, 1E+8 };
while (tt_args->remaining > 0) {
nanosleep(&time, NULL);
pthread_mutex_lock(&tt_args->mutex);
if (!tt_args->paused) {
tt_args->remaining -= (time.tv_nsec / 1e6);
}
pthread_mutex_unlock(&tt_args->mutex);
}
tt_args->post(args);
return NULL;
}

void toggle_pause(TimerThread* tt)
{
pthread_mutex_lock(&tt->mutex);
tt->paused = !tt->paused;
pthread_mutex_unlock(&tt->mutex);
}

void start(TimerThread* tt)
{
tt->paused = 0;
tt->stopped = 0;
tt->remaining = tt->total;
pthread_create(&tt->thread, NULL, StartTimerThread, tt);
}

void stop(TimerThread* tt)
{
tt->stopped = 1;
pthread_cancel(tt->thread);
tt->remaining = tt->total;
}
```

2 Answers

Answered By CodeWiz007 On

It's cool that you're diving into multi-threading with C! Just an FYI, using both `#pragma once` and include guards in your header file isn’t necessary. Pick one for consistency.

TimerTinkerer -

Right? It’s often recommended to stick with include guards since `#pragma once` isn’t part of the C standard. Better safe than sorry!

CleverKoala42 -

Good point! I’ll go with include guards, seems more reliable.

Answered By DebugDude On

I noticed your function signatures are inconsistent; you're using different names in the header and the implementation. Since you know you'll always be using `TimerThread*`, you might as well simplify your function signature to take it directly instead of casting.

CodeWiz007 -

But I think he needs to follow the `pthread_create` requirements, which expect a `void*` parameter for the thread start function.

CleverKoala42 -

Exactly! That's why I’m casting it in my implementation.

Related Questions

LEAVE A REPLY

Please enter your comment!
Please enter your name here

This site uses Akismet to reduce spam. Learn how your comment data is processed.