Timer application using GTK

Question

Purpose

The code is for a timer with a starting time of 25 minutes. The goal is to have a very simple pomodoro application with Start, Pause and Reset. Also you can tick a box to make the UI stay in front of the desktop so you always have the timer in front of your eyes.

Hierarchy

main.c
 /src
 structures.h
 /features
 timer.c
 timer.h
 /frontend
 frontend.c
 frontend.h
 style.css
 view.glade
 /signals
 options.c
 options.h
 timer_buttons.c
 timer_buttons.h

Code

main.c

#include <stdlib.h>
#include <gtk/gtk.h>
#include <time.h>
#include <glib.h>
#include <stdbool.h>
#include "src/structures.h"
#include "src/signals/options.c"
#include "src/signals/timer_buttons.c"
#include "src/frontend/frontend.c"
#include "src/features/timer.c"
int
main (int argc,
 char **argv)
{
 GtkBuilder *builder;
 GtkWidget *window;
 struct TimerUI timerUi;
 bool success;
 gtk_init(&argc, &argv);
 builder = gtk_builder_new();
 // set up user interface
 success = init_view(builder);
 if (!success)
 return 1;
 // get and set up window
 window = GTK_WIDGET( gtk_builder_get_object (builder, "window"));
 timerUi.window = GTK_WINDOW(window);
 g_signal_connect(window, "destroy", G_CALLBACK(gtk_main_quit), NULL);
 gtk_window_set_title(GTK_WINDOW(window), "Pomodoro Timer");
 // connect signals to builder
 gtk_builder_connect_signals(builder, &timerUi);
 // timer
 init_timer(builder, &timerUi);
 // style
 success = init_style(&timerUi);
 if (!success)
 return 1;
 // start
 gtk_widget_show(window);
 gtk_main();
 return 0;
}

src/structures.h

#ifndef STRUCTURES_H_INCLUDED
#define STRUCTURES_H_INCLUDED
struct TimerUI
{
 GtkWindow *window;
 GtkLabel *label;
 int hours;
 int seconds;
 int timerReference;
};
#endif // STRUCTURES_H_INCLUDED

src/features/timer.c

#include "timer.h"
#include "../structures.h"
void
init_timer(GtkBuilder *builder,
 struct TimerUI *timerUi)
{
 // ui
 timerUi->label = GTK_LABEL(gtk_builder_get_object(builder, "timer"));
 // timer callback reference
 timerUi->timerReference = 0;
 // reset
 reset_timer(timerUi);
}
void
reset_timer(struct TimerUI *timerUi)
{
 timerUi->hours = 25;
 timerUi->seconds = 0;
 gtk_label_set_text(timerUi->label, "25:00");
}
gboolean
run_timer(struct TimerUI *timerUi)
{
 char formattedTime[6];
 char formattedSeconds[3];
 if (timerUi->seconds == 0)
 timerUi->seconds = 59;
 else
 timerUi->seconds = timerUi->seconds - 1;
 if (timerUi->seconds == 59)
 timerUi->hours = timerUi->hours - 1;
 if (timerUi->seconds < 10 && timerUi->seconds >= 0)
 snprintf(formattedSeconds, 3, "0%d", timerUi->seconds);
 else
 snprintf(formattedSeconds, 3, "%d", timerUi->seconds);
 snprintf(formattedTime, 6, "%d:%s", timerUi->hours, formattedSeconds);
 gtk_label_set_text(GTK_LABEL(timerUi->label), formattedTime);
 return true;
}

src/features/timer.h

#ifndef TIMER_H_INCLUDED
#define TIMER_H_INCLUDED
void init_timer (GtkBuilder *builder,
 struct TimerUI *timerUi);
void reset_timer (struct TimerUI*);
gboolean run_timer (struct TimerUI*);
#endif // TIMER_H_INCLUDED

src/frontend/frontend.c

#include "frontend.h"
#include "../structures.h"
bool
init_view(GtkBuilder *builder)
{
 GError *error = NULL;
 // tricky way to make it work once installed but also if you compile manually
 if (gtk_builder_add_from_file(builder, "/usr/local/share/main/view.glade", &error) == 0) {
 if (gtk_builder_add_from_file(builder, "src/frontend/view.glade", &error) == 0) {
 g_warning("%s", error->message);
 g_clear_error(&error);
 return false;
 }
 }
 return true;
}
bool
init_style(struct TimerUI *timerUi)
{
 GtkStyleContext *context;
 GtkCssProvider *provider;
 GError *error = NULL;
 context = gtk_widget_get_style_context(GTK_WIDGET(timerUi->label));
 provider = gtk_css_provider_new();
 // tricky way to make it work once installed but also if you compile manually
 gtk_css_provider_load_from_path(provider, "/usr/local/share/main/style.css", &error);
 if (error) {
 gtk_css_provider_load_from_path(provider, "/usr/local/share/main/style.css", &error);
 if (error) {
 g_warning("%s", error->message);
 g_clear_error(&error);
 return false;
 }
 }
 gtk_style_context_add_provider(context, GTK_STYLE_PROVIDER(provider), GTK_STYLE_PROVIDER_PRIORITY_USER);
 return true;
}

src/frontend/frontend.h

#ifndef FRONTEND_H_INCLUDED
#define FRONTEND_H_INCLUDED
bool init_view (GtkBuilder *builder);
bool init_style (struct TimerUI *timerUi);
#endif // FRONTEND_H_INCLUDED

src/signals/options.c

#include "options.h"
#include "../structures.h"
void stick_checkbox_signal(GtkToggleButton *checkBox, struct TimerUI *timerUi)
{
 bool isActive = gtk_toggle_button_get_active(checkBox);
 gtk_window_set_keep_above(timerUi->window, isActive);
}

src/signals/options.h

#ifndef OPTIONS_H_INCLUDED
#define OPTIONS_H_INCLUDED
void stick_checkbox_signal (GtkToggleButton*,
 struct TimerUI*);
#endif // OPTIONS_H_INCLUDED

src/signals/timer_buttons.c

#include "timer_buttons.h"
#include "../structures.h"
#include "../features/timer.h"
void
start_signal(GtkWidget *widget,
 struct TimerUI *timerUi)
{
 if (timerUi->timerReference != 0)
 return;
 timerUi->timerReference = g_timeout_add(1000, G_SOURCE_FUNC(run_timer), timerUi);
}
void
pause_signal(GtkWidget *widget,
 struct TimerUI *timerUi)
{
 if (timerUi->timerReference != 0)
 g_source_remove(timerUi->timerReference);
 timerUi->timerReference = 0;
}
void
reset_signal(GtkWidget *widget,
 struct TimerUI *timerUi)
{
 if (timerUi->timerReference != 0)
 g_source_remove(timerUi->timerReference);
 timerUi->timerReference = 0;
 reset_timer(timerUi);
}

src/signals/timer_buttons.h

#ifndef TIMER_BUTTONS_H_INCLUDED
#define TIMER_BUTTONS_H_INCLUDED
void start_signal (GtkWidget*,
 struct TimerUI*);
void pause_signal (GtkWidget*,
 struct TimerUI*);
void reset_signal (GtkWidget*,
 struct TimerUI*);
#endif // TIMER_BUTTONS_H_INCLUDED

src/frontend/view.glade

?xml version="1.0" encoding="UTF-8"?>
<!-- Generated with glade 3.22.2 -->
<interface>
 <requires lib="gtk+" version="3.20"/>
 <object class="GtkWindow" id="window">
 <property name="can_focus">False</property>
 <property name="window_position">center</property>
 <child type="titlebar">
 <placeholder/>
 </child>
 <child>
 <object class="GtkGrid">
 <property name="visible">True</property>
 <property name="can_focus">False</property>
 <property name="margin_left">3</property>
 <property name="margin_right">3</property>
 <property name="margin_top">3</property>
 <property name="margin_bottom">3</property>
 <property name="column_homogeneous">True</property>
 <child>
 <object class="GtkButton" id="bottomButtonReset">
 <property name="label" translatable="yes">Reset</property>
 <property name="visible">True</property>
 <property name="can_focus">True</property>
 <property name="receives_default">True</property>
 <property name="margin_left">6</property>
 <property name="margin_right">6</property>
 <property name="margin_top">6</property>
 <property name="margin_bottom">6</property>
 <signal name="clicked" handler="reset_signal" swapped="no"/>
 </object>
 <packing>
 <property name="left_attach">3</property>
 <property name="top_attach">12</property>
 </packing>
 </child>
 <child>
 <object class="GtkButton" id="bottomButtonPause">
 <property name="label" translatable="yes">Pause</property>
 <property name="visible">True</property>
 <property name="can_focus">True</property>
 <property name="receives_default">True</property>
 <property name="margin_left">6</property>
 <property name="margin_right">6</property>
 <property name="margin_top">6</property>
 <property name="margin_bottom">6</property>
 <signal name="clicked" handler="pause_signal" swapped="no"/>
 </object>
 <packing>
 <property name="left_attach">1</property>
 <property name="top_attach">12</property>
 </packing>
 </child>
 <child>
 <object class="GtkButton" id="bottomButtonStart">
 <property name="label" translatable="yes">Start</property>
 <property name="visible">True</property>
 <property name="can_focus">True</property>
 <property name="receives_default">True</property>
 <property name="margin_left">6</property>
 <property name="margin_right">6</property>
 <property name="margin_top">6</property>
 <property name="margin_bottom">6</property>
 <signal name="clicked" handler="start_signal" swapped="no"/>
 </object>
 <packing>
 <property name="left_attach">2</property>
 <property name="top_attach">9</property>
 <property name="height">2</property>
 </packing>
 </child>
 <child>
 <object class="GtkCheckButton">
 <property name="label" translatable="yes">Stick on screen</property>
 <property name="visible">True</property>
 <property name="can_focus">True</property>
 <property name="receives_default">False</property>
 <property name="margin_left">6</property>
 <property name="margin_right">6</property>
 <property name="margin_top">6</property>
 <property name="margin_bottom">6</property>
 <property name="draw_indicator">True</property>
 <signal name="toggled" handler="stick_checkbox_signal" swapped="no"/>
 </object>
 <packing>
 <property name="left_attach">1</property>
 <property name="top_attach">14</property>
 </packing>
 </child>
 <child>
 <object class="GtkLabel" id="timer">
 <property name="name">timer-text</property>
 <property name="visible">True</property>
 <property name="can_focus">False</property>
 <property name="margin_left">6</property>
 <property name="margin_right">6</property>
 <property name="margin_top">6</property>
 <property name="margin_bottom">6</property>
 <property name="hexpand">True</property>
 <property name="vexpand">True</property>
 </object>
 <packing>
 <property name="left_attach">1</property>
 <property name="top_attach">1</property>
 <property name="width">3</property>
 <property name="height">7</property>
 </packing>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 <child>
 <placeholder/>
 </child>
 </object>
 </child>
 </object>
</interface>

src/frontend/style.css

#timer-text {
 font: 60px "Comic Sans";
}

Compile

I compile the program on Ubuntu 20.04.1 using:

gcc-9 `pkg-config --cflags gtk+-3.0` -o main main.c `pkg-config --libs gtk+-3.0` -rdynamic -g -Wall

Question

Outside of the overall design of that C program, am I missing anything about security or performance? I’m not doing any memory management really, I wonder if I should (or maybe I misunderstand what it means). Basically, I wonder if I am missing things you need to do in C since I usually code with high-level programming languages.

• pacmaninbw: Welcome to the Code Review Community. You will get a better code review if you include the complete files you want reviewed. Right now your code is missing at least the header file include statements.
• gliese667cc: Ok, thanks. That should be all the complete files now.

Answer 1

Here are some things that may help you improve your program.

Reconsider the directory structure

I generally prefer a flat data structure for programs as small and simple as this one. That is, I'd put all of the code in src including main.c. This makes maintenance a bit easier because we no longer need to include the directory structure in every single source file that uses an #include. For more complex projects where I do use different directories, I tend to tell the compiler where to find the #include files rather than having that information hardcoded in each file. That way, I have a single place (e.g. a Makefile or a CMakeLists.txt file) that stores the hierarchy.

Make sure you have all required #includes

The code uses bool in several places but doesn't #include <stdbool.h>. Also, a number of the include files are missing items. For instance, the timer.h file is missing these:

#include "structures.h"
#include <gtk/gtk.h>

Don't hardcode file names

Generally, it's not a good idea to hardcode a file name in software, and generally especially bad if it's an absolute file name (as contrasted with one with a relative path). Instead, it would be better to allow the user of the program to specify the name, as with a command line parameter or a configuration file.

Only #include header files

The main.c code incorrectly includes other .c files. What it should do instead is only include the .h files (the interface files). Each .c file is converted into an object file .o and then they are linked into a single excutable. I use CMake typically, so one way to write a CMakeLists.txt file for the top level of this project is this:

cmake_minimum_required(VERSION 3.1)
project(pomodoro)
set(CMAKE_C_STANDARD 11)
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -pedantic")
add_subdirectory(src)

Then within the src directory, another CMakeLists.txt file looks like this:

cmake_minimum_required(VERSION 3.1)
find_package(PkgConfig REQUIRED)
pkg_check_modules(GTK3 REQUIRED gtk+-3.0)
include_directories(${GTK3_INCLUDE_DIRS})
link_directories(${GTK3_LIBRARY_DIRS})
add_definitions(${GTK3_CFLAGS_OTHER})
add_executable(pomodoro "timer_buttons.c" "options.c" "frontend.c" "timer.c" "main.c")
target_link_libraries(pomodoro ${GTK3_LIBRARIES})

Avoid duplicating contants

The code currently includes this function:

void
reset_timer(struct TimerUI *timerUi)
{
 timerUi->hours = 25;
 timerUi->seconds = 0;
 gtk_label_set_text(timerUi->label, "25:00");
}

If I wanted to change this to be, for example, a 15 minute timer, I'd have to make that change twice. What I'd suggest instead is to create a function to format the string and then call it from here and also from within run_timer. Even better, make it configurable by the user. Also, that should clearly be minutes and not hours!

Simplify formatting

The code currently has this for formatting the timer string:

char formattedTime[6];
char formattedSeconds[3];
if (timerUi->seconds < 10 && timerUi->seconds >= 0)
 snprintf(formattedSeconds, 3, "0%d", timerUi->seconds);
else
 snprintf(formattedSeconds, 3, "%d", timerUi->seconds);
snprintf(formattedTime, 6, "%d:%s", timerUi->hours, formattedSeconds);

That's much more complex than needed:

char formattedTime[6];
snprintf(formattedTime, 6, "%d:%2.2d", timerUi->minutes, timerUi->seconds);
gtk_label_set_text(GTK_LABEL(timerUi->label), formattedTime);

In keeping with the previous suggestion, I'd factor it out like this:

static void 
set_timer_string(struct TimerUI *timerUi) 
{
#define TIMESTRINGLEN 6
 char formattedTime[TIMESTRINGLEN];
 snprintf(formattedTime, TIMESTRINGLEN, "%d:%2.2d", timerUi->minutes, timerUi->seconds);
 gtk_label_set_text(GTK_LABEL(timerUi->label), formattedTime);
}

Think of the user

If style.css fails to load or the view.glade file has an error in it, the user is given a rather cryptic error message and then the program abruptly halts. It would be better to gracefully capture these and report useful error messages.

It's also a bit confusing that when the timer expires it shows "-1:59" and keeps going. I'd have expected that it stops, or turns red or something to indicated that it has expired.

• gliese667cc: Sorry for the very late reply, you made very good points. I really lack time and I have moved to another project so I didn't take everything. I have implemented the "Avoid duplicating contants" and "Make sure you have all required #includes" so far and also fixed the negative timer bug. I'm not sure if we're allowed to share links but I have released the code github.com/lalande21185/pomodoro-timer.