[Gossip-dev] [Telepathy] rework of the UI

[Eitan Isaacson]

On Wed, 2006-09-13 at 19:09 +0200, Xavier Claessens wrote:
> Le mercredi 13 septembre 2006 à 18:56 +0200, Richard Hult a écrit :
> > >> * g_value_transform looks suspicious, is that right? don't you just want
> > >> to copy the value?
> > > g_value_transform copy the value and make some casting too. For example
> > > it accept to copy a g_value containing a G_TYPE_UINT to G_TYPE_INT. I
> > > think g_value_copy will make a warning if types are not exactly the
> > > same.
> > 
> > I'd say that since this is completely internal to libgossip, using the
> > wrong types here is a programming bug, so if we make this stricter
> > that's just a good thing in my opinion.
> 
> With telepathy it won't be completly internal since those values will be
> given by telepathy... I'm not absolutely sure about that, Eitan can
> confirm ?

The reason I put transform instead of copy is for it to be easier to
take strings from the xml file or text fields, and easily transform them
to guint and gint GValues, and vice versa. For example, port numbers.
But since we have utility functions that do this properly, and since
Xavier promoted them to libgossip, I don't see any need for it.

> 
> > >> * ...what's the reason really to have a datalist, when a hashtable would
> > >> work just as fine and probably be simpler and less code?
> > > Seems almost the same, Eitan Isaacson told me he would like to use
> > > g_datalist. As I understand g_datalist can be more efficient because it
> > > searches values using a GQuark (integer value) instead of a string...
> > > but that's almost a matter of s/g_datalist_get_data/g_hash_table_lookup/
> > 
> > Performance will never be an issue for this kind of thing, code
> > clarity/simplicity is worth a lot more. Unless you plan on looking up
> > 100 000 params in a tight loop or something :)
> 
> I agree, Eitan and Martyn told me to use datalist but I'm not sure it's
> the best way... Eitan what's your opinion by reading the code ?

It looks clear to me, I don't see why it complicates anything, the docs
convinced me to use it since they say that is how 
g_object_set_data() and friends are implemented, and since we are
dealing with dynamic properties, I feel it is appropriate.

A dilemma I had when I wrote the original hashtable was that if we
choose not to expose the hashtable and GossipAccountParam, we will need
to do 2 or 3 lookups at a time for each parameter when it is marshaled
or when we build an appropriate UI on the fly (value and flags). I know
it is not a real performance issue, it just seems like bad form to me. 

I'm not sure GQuarks are any more efficient, since we create a new
GQuark every time, and although I haven't looked, g_str_equal probably
works the same way anyway.
Bottom line: I don't see any immediate benefit, but I have a gut feeling
we will be glad we did this in the future :)

> 
> Xavier.
> _______________________________________________
> Gossip-dev mailing list
> Gossip-dev at lists.imendio.com
> http://lists.imendio.com/mailman/listinfo/gossip-dev