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