Re: GHashTable improvements

[Tim Janik <timj gtk org>]

On 16 Mar 2001, Owen Taylor wrote:

> I was looking back at Sven's patches in #50796 to see if
> they were ready to commit. The patch adds:
> 
> =======
> GHashTable* g_hash_table_new_full          (GHashFunc       hash_func,
> 					    GEqualFunc      key_equal_func,
> 					    GDestroyNotify  key_destroy_func,
> 					    GDestroyNotify  value_destroy_func);
> void        g_hash_table_destroy_no_notify (GHashTable     *hash_table);
> void        g_hash_table_replace           (GHashTable     *hash_table,
> 					    gpointer        key,
> 					    gpointer        value);
> gboolean    g_hash_table_remove_no_notify  (GHashTable     *hash_table,
> 					    gconstpointer   key);
> =======
> 
> The items that may be outstanding here are:
> 
>  - In the discussion earlier, there was some idea that g_hash_table_replace()
>    wasn't necessary as long as g_hash_table_insert() called the destroy
>    notify on the key function.

that would change the documented semantics of g_hash_table_insert() in a way
that's very hard to debug. we had this discussion (on gnome-hackers iirc) during
pre-1.2 and didn't change it back then either. 
so g_hash_table_insert() should stay as it is, i.e. not touching node->key if
it's used to replace items, even if that justifies another API call
g_hash_table_replace(). at least with that, we can tell people to use
g_hash_table_replace() if they want to replace instead of insert.
the replacement bahviour for _insert() just needs to be properly explained
in the official docs and that's it.

>    The main argument for keeping g_hash_table_replace() then seems
>    to be that you might have a case where key and value are
>    associated:
> 
>     g_hash_table_insert (hash, entry->name, entry);
>   
>    I've done this fairly frequently in the past.
> 
>  - Do we need g_hash_table_foreach_remove_no_notify() since
>    we have remove_no_notify() and destroy_no_notify()?

i would consider the dataset stealing API a fairly special case, and
don't think it's such a great idea to add that everywhere we support
destroy notifiers. so i personally would rather not see these _no_notify
variants.

>  - The name in GObject is not g_object_set_data_notify() as it
>    was in GtkObject, but g_object_steal_data(). So perhaps,
>    if this is our desired name, we should have
>    g_hash_table_steal() g_hash_table_destroy_stealing_all (????)
>    g_hash_table_foreach_steal()? 

it's not quite the same, will g_hash_table_remove_no_notify(), while
returning the undestroyed value, still destroy the key?
yes? then we also need g_hash_table_remove_no_notify_any(). ;)
no?  then we also need g_hash_table_remove_value_no_notify(). ;)
i'd rather not have no_notify variants for GHashTable, did i say that already?

> Other than these items, and the addition of docs, I think it is a useful 
> addition, and about ready to commit.
> 
> Regards,
>                                         Owen
> 
> 

---
ciaoTJ