On Wed, Nov 25, 2015 at 6:53 AM, Lubomir I. Ivanov <[email protected]> wrote: >> subsurface-core/divelist.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/subsurface-core/divelist.c b/subsurface-core/divelist.c >> index a14fabf..a2e94c0 100644 >> --- a/subsurface-core/divelist.c >> +++ b/subsurface-core/divelist.c >> @@ -790,6 +790,9 @@ void add_single_dive(int idx, struct dive *dive) >> dive_table.nr++; >> if (dive->selected) >> amount_selected++; >> + if (idx < 0) >> + // convert an idx of -1 so we do insert-at-end: >> + idx = dive_table.nr - 1; >> for (i = idx; i < dive_table.nr; i++) { >> struct dive *tmp = dive_table.dives[i]; >> dive_table.dives[i] = dive; > > currently add_single_dive() assumes a safe index. > i think that add_single_dive() should not be touched, but instead the > mobile app should be fixed (models bug?). >
That sounds reasonable, but i would mention the following: 1. if add_single_dive is going to assume a safe index, then we should add an assert. an assertion failure would have been preferable to varied and mysterious crashes after the call. 2. the ability to call with -1 is also a feature. currently, if you want to add a dive to the *front* of the list, you pass 0. But if you want to add it to the *end* of the list, you need to know the size of the table (so you can pass the table size as your targeted insertion point). Passing 0 to mean "prepend" does not require knowledge of the current table size. So, using -1 as a shorthand for "append" provides a similar feature, then. no need to know the table size. /K _______________________________________________ subsurface mailing list [email protected] http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
