On Wednesday 19 December 2012 00:00:27 Ivan Stankovic wrote: > On Mon, Dec 17, 2012 at 04:31:43PM +0000, Peter TB Brett wrote: > > #define EDA_TYPE_OBJECTSET (eda_object_set_get_type ()) > > ^ > You probably want to remove that underscore. > > Also, EdaObjectset looks a bit weird, compared to naming conventions > in other libraries, but I don't have a better name.
It does. :-( If we were using a "proper" OO system, I don't think that
we would be worrying about this (look at the Java Collections API "Set"
interface for example).
> > struct _EdaObjectsetClass
> > {
> >
> > GObjectClass parent_class;
> >
> > /* virtual public methods */
> > gboolean (*get_iter)(EdaObjectset *set, EdaObjectIter *iter, gint
> > index);
> I assume the index here is the initial iterator position.
Yes.
> > gboolean (*find)(EdaObjectset *set, EdaObjectIter *iter);
>
> Missing OBJECT * here.
Good point.
> > OBJECT *(*get)(EdaObjectset *set, EdaObjectIter *iter);
> > void (*append)(EdaObjectset *set, OBJECT *object);
> > void (*insert)(EdaObjectset *set, EdaObjectIter *iter, OBJECT
> > *object); void (*remove)(EdaObjectset *set, EdaObjectIter *iter,
> > OBJECT *object);
> Looking good (but I wonder about subclasses and how they might
> implement these).
I introduced "append" as distinct from "insert" because it's impossible
to obtain an iterator pointing beyond the end of the set.
> > /* Convenience functions */
> > EdaObjectset *eda_objectset_new (void) G_GNUC_WARN_UNUSED_RESULT;
> > void eda_objectset_foreach (EdaObjectset *set, GCallback func,
> > gpointer user_data);
> >
> > /* Getting iterators */
> > gboolean eda_objectset_get_iter (EdaObjectset *set, EdaObjectIter
> > *iter, gint index); gboolean eda_objectset_iter_next (EdaObjectset
> > *set, EdaObjectIter *iter); gboolean eda_objectset_find
> > (EdaObjectset *set, EdaObjectIter *iter, OBJECT *object);
> >
> > /* Obtaining values */
> > OBJECT *eda_object_set_get (EdaObjectset *set, EdaObjectIter *iter);
>
> ^
> Extra undescore.
>
> > /* Modifying the set */
> > void eda_objectset_append (EdaObjectset *set, OBJECT *object);
> > void eda_objectset_insert (EdaObjectset *set, EdaObjectIter *iter,
> > OBJECT *object); void eda_objectset_remove (EdaObjectset *set,
> > EdaObjectIter *iter);
> Looking good, but I expect the eda_objectset_remove method will be
> cumbersome to use. You would first need to obtain the iterator that
> points to the object and then call eda_objectset_remove. For example,
> eda_objectset_append does not require an iterator.
Okay, so as mentioned above, "append" probably should require an
iterator -- it'll append an object to the set and then set the iterator
to point to the new location in the set.
> APIs are best judged by examples so let's say that I want to remove
> all objects from a page that satisfy some criteria:
>
> EdaObjectIter iter;
> EdaObjectset *set = eda_page_get_objects(page);
>
> if(eda_objectset_get_iter(set, &iter, 0)) {
> while(1) {
> EdaObject *obj = eda_objectset_get(set, &iter);
>
> if(should_remove(obj))
> eda_objectset_remove(set, &iter);
>
> if(!eda_objectset_iter_next(set, &iter))
> break;
> }
> }
>
>
> While this is not too verbose, I have a feeling we could do better.
Yes,
> More troublesome is that it's not obvious how eda_objectset_remove
> and eda_objectset_iter_next interact. Does eda_objectset_remove
> automatically advances the iterator? Does the iterator get
> invalidated?
Yes, "remove" will make sure that the iterator is updated to point to
the entry after the one which was removed. That means that it's actually
*possible* to remove all objects that satisfy particular criteria from
the set in O(N) rather than O(N^2) time without making a copy of the
set. For example, how would you implement the algorithm above using an
eda_objectset_remove() that doesn't use an iterator?
This discussion all seems a bit daft... I feel like this is exactly the
sort of thing that should be provided by a basic library.
Peter
--
Peter Brett <[email protected]>
Remote Sensing Research Group
Surrey Space Centre
signature.asc
Description: This is a digitally signed message part.
-- Mailing list: https://launchpad.net/~geda-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~geda-developers More help : https://help.launchpad.net/ListHelp
