working towards Subsurface 2.0

Dirk Hohndel dirk at hohndel.org
Sun Sep 9 19:37:22 PDT 2012


Miika Turkia <miika.turkia at gmail.com> writes:
>>>>> Do you want this included in subsurface? Should I go ahead and include
>>>>> the trip stats?
>>>>
>>>> How are you planning to offer those to the user?
>>>
>>> The current implementation is a new window that shows the yearly stats
>>> in list view, invoked from menu.
>>
>> I have a hard time visualizing that. I'd love a pull request :-)
>
> I have a patch attached. If pull requests are the preferred way I'll
> take a look into github for the future.
> The yearly statistics is currently invoked from the menu Log->Yearly Statistics.

I find myself really liking this - a great addition to the statistics
that we already have. Thanks for the great work!

Couple of comments on the code:

Don't use <stdbool.h> and bool - we already get gboolean from
Gtk. And try to use TRUE and FALSE with boolean values.

This seems a bit complicated:

+static char * get_minutes(int seconds)
+{
+       static char buf[80];
+       int minutes = seconds / 60;
+       int sec = seconds % 60;
+       snprintf(buf, sizeof(buf), "%d:%.2d", minutes, sec);
+       return buf;
+}

Look at the FRACTION macro that we already have in save-xml.c
Maybe you could move that to a .h file and reuse here.

The code in process_interval_stats looks like it could benefit from a
clever macro that hides the repetitive code and brings out the actual
flow of the logic.

key_press_event should at least also respond to Ctrl-W (or Command-W on
a Mac - but that's transparent in the way we use Gtk on the Mac).

the logic with init_yearly is simply wrong - if we add dives while
subsurface is running (either by importing or by manually adding dives)
you don't update the statistics. process_all_dives is called when
subsurface thinks that the statistics need to be reevaluated. So simply
reallocate your data structures and recalculate the statistics.

do the stats_yearly == NULL check outside of the loop over the
dive_table (no point to constantly check that).

I'd love to see a pull request with these changes (assuming Linus has no
additional comments)

/D


More information about the subsurface mailing list