Greg Ercolano wrote:
> I often abuse Fl_Widget::user_data() for such purposes,
> but this sounds better.
Yup.
> It looks like you were able to remove the tooltip pointer and
> replace it with the more flexible 'Fl_Widget_Association'
>
> The downside is that the association table is static, and
> therefore grows with each widget added, causing 'linear lookup'
> overhead for each widget that has association data.
That's only partially true. There's a hash table, so that a widget's
"Association Node" can be found by only a modulo calculation and a
direct table lookup. However, there can be more than one widget with
the same hash value, and thus it is correct that there can be a linear
search through the linked list of more than one widget's association
entries. However, the table is always at least as big as the number
of widgets that have associations (IIRC), and thus I don't think that
there will be a lot of widgets with the same hash value in general,
but that may depend...
> Possibly each widget could have a single association pointer
> (ie. replace the tooltip* with an Fl_Association*, so the Fl_Widget's
> don't actually need to get bigger),
I wouldn't recommend to replace the tooltip pointer. IMHO the tooltip
replacement was only intended as an example. I don't think that it would
be useful (in terms of performance) to replace any core widget property
with the Fl_Widget_Association mechanism.
Thus, we would have one more pointer for as many associations per widget
as a user would like. That sounds good to me...
> and this pointer would be NULL
> if there's no associations, and would keep track of its own size,
> and grow one element for each association added. This way any lookups
> would be on the small per-widget array, instead of a lookup into a
> globally static table. The array This would mean the association stuff
> would need to be public, but that's maybe for the good..?
Not "more public" than with the proposed solution, IMHO.
But we would lose the ability of Andreas's solution to find all widgets
that have a particular association type, since there wouldn't be a central
storage anymore. I don't think that this would be a big loss, though.
In fact, I can't think of an application that would need this. Even the
mentioned layout manager could use the normal widget group hierarchies
to iterate the widgets and request each widget's layout (association)
info.
Andreas or others: does anyone know of an example where we would need to
search (or iterate with the proposed "foreach" functions) all widgets
with at particular association type?
> Or maybe I'm missing some aspect of the suggested code, not sure.
> For sure, whether the data within is a linked list or an array
> should probably be invisible to the caller.
Yup, agreed. And both would be possible.
> I would request a few naming changes for consistency before accepting
> the patch:
>
> > Change the abbreviation "Ass" to "Assoc", so it's clear what
> the abbreviation stands for. ("Ass" is funny, but unclear)
>
> > Consistent use of caps in naming.
> For instance "Tooltip" instead of "ToolTip" to be consistent
> with existing "Fl_Tooltip.H"
>
> ToolTipAssociationType -> TooltipAssociationType
>
> ..and Fl_Word_Word_Word underbar naming for public classes,
> eg:
>
> Fl_AssociationType -> Fl_Association_Type
>
> > FLTK oriented naming conventions for methods/macros, eg:
>
> static void growTable(void)-> static void
> growtable(void) // eg.
> void Fl_Widget::AssAdd() -> void
> Fl_Widget::add_assoc() // eg. add_fd(), add_idle(), etc
> bool Fl_Widget::AssRemove -> bool
> Fl_Widget::remove_assoc() // eg. remove_fd(), remove_idle(), etc
> fltk_WidgetAssociation_h -> Fl_Widget_Association_H
> // eg. Fl_Preferences_H, Fl_Printer_H, etc
>
> > Carefully follow the FLTK CMP/ doxygen coding standards, eg.
> http://docs.google.com/Doc?id=dgn42tzv_0fsbjmjgp
> /*!
> * Add an association to a this widget. The associated data is of the given
> * association type. The associated data must not be NULL. NULL is simply
> * not added as association.
> */
> void Fl_Widget::AssAdd(const Fl_AssociationType& at, void* data) const {
>
>
> ..to..
>
> /**
> Add an association to a this widget. The associated data is of the given
> association type. The associated data must not be NULL. NULL is simply
> not added as association.
> \param[in] at The association type to be modified <<
> \param[in] data The new association data to be added <<
> */
> void Fl_Widget::add_assoc(const Fl_Association_Type& at, void* data) const {
Agreed. But that's secondary, IMHO. We should discuss if we want or even
need such an extension. I believe that FLTK could benefit from it.
We should also consider that this proposal is a backport from FLTK 2.
Once we start the fusion of FLTK versions (aka FLTK 3), we would need
to implement / copy this anyway. So why not start it right now ?
Albrecht
_______________________________________________
fltk-dev mailing list
[email protected]
http://lists.easysw.com/mailman/listinfo/fltk-dev
