[PATCH 2/2] tree_selected_foreach: ingore trips in the selection

[Dirk Hohndel]

"Lubomir I. Ivanov" <neolit123 at gmail.com> writes:

> On 28 September 2012 08:17, Dirk Hohndel <dirk at hohndel.org> wrote:
>>
>> Ok, here is a quick and dirty, virtually untested patch that does a linear time walk of the dive table and should do everything we need with the exception of maintaining the expanded state of the trips (easy enough to add).
>>
>> Please give it a look and let me know what you think.
>>
>
> one thing i've noticed, the iterator needs to move back when we shift
> the dive list within the main loop in delete_selected_dives(), so that
> selected dives are not skipped (i--).

Logic says it does - yet one of the two small tests that I did before
sending this out was deleting all dives in a trip to make sure that it
disappeared. I wonder how that ended up working. :-)

> this patch (attachment) add the above to yours and also replaces the
> delete_single_dive() with the new one, that can also be reused in
> delete_dive_cb().
>
> however delete_dive_cb() also needs to update the parent trip ("when"
> flag) and the expanded states.

Definitely. But again, that's quite simple to do with the infrastructure
that we have. Remember, every time you delete something the Gtk model
gets thrown away and rebuilt, so all you need to maintain is the
dive_trip_list... 

> i wonder if delete_dive_cb() and delete_selected_dives_cb() should
> only delete the dives (via delete_singe_dive()) and store a per
> dive_trip "needs_update" flag, for the trip from which dives were
> deleted.

Well, since they KNOW which trip they are in and can easily detect if
they are the relevant dive in the trip (by being the one where the
timestamps match), this is easy, too, right?

> delete_selected_dives() still has to iterate trough the entire list to
> collect the selected dives unless we have the selected dives pointers
> allocated in a dynamic buffer, which can be controlled via
> gtk_tree_selection_set_select_function().

But think about it. Iterating once through the entire list is blazing
fast. It's an array with at worst a few thousand entries. That takes no
time at all compared to all the crud and overhead of traversing the Gtk
model.

> dive_list_update_dives() calls fill_dives() which also iterates trough
> the entire list again, so perhaps it should take care of which dives
> were previously selected, which trips have to be expanded and updated
> before the model is recreated.

I am loath to make the fill_dive function even more complex. It's bad
enough as it is. Again, as long as you are talking about walking the
array this is really low overhead.

> ----
>
> another thing which i would have definitely have done is to have a
> better dive<->trip control, by having a "int total_dives" and "struct
> dive **divelist" members of struct dive_trip, where "divelist" is a
> dynamic buffer storing pointers to all dives children of the said trip
> (or alternatively storing their indexes - int *diveindexes, given that
> the divelist is a public variable). then when a trip "needs_update" it
> is only required to iterate trough these children and grab the
> smallest "when" flag. perhaps, later on not only the "when" flag has
> to be updated, but also other properties of a trip
> (cough...images...:))

I know, people want that. But I disagree with your statement that we
need a data structure to hold the children. That's just something we
need to maintain - and we have all the information that we need.

The dive_table is chronologically ordered. So if you have a dive_trip
find the dive that matches it's timestamp and keep walking forward in
the list until you either have a dive that is not in a trip or you have
a dive with a timestamp matching the next trip. All this is O(n) and
given how small n is that means it's very simple.

The overhead (and risk of errors) in maintaining a complicated
structure... no way.

And as to storing other 'assets'. I'd do that per dive and simply have
everything else in a database (who knows what people will end up wanting
to store) and then store a unique key into that database in the dive or
the dive trip (depending what you want to associate those assets with).

> excluding the potential downsides of how reliable a mem. mapper is for
> small chunks, i was recently working on something that pretty much
> forced me to do some of the explained above, because:
> 1) the application API was not convenient without it in regard of
> child<->parent object communication.
> 2) i had to iterate trough many values to get a single object flag

In this case here I think that would be total overkill.

/D