Code review comment for lp:~kamstrup/libunity/faves-enumerator

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

> > I thought quite a lot about this when writing the API. The idea was that
> > get_instance() had the semantics of a "pure" singleton, which you can only
> > ever have precisely one of; while get_default() implies that you can have
> > different instances, but normally just one.
>
> You can't really have "pure" singleton with gobject (g_object_new (...)),
> that's why everyone uses get_default.

Sure you can, just override GObject->constructor(), but that aside. We agree on the solution :-)

> > In the same spirit I think we should rename
> > Unity.AppInfoManager.get_instance() as well - although that may be too hard
> an
> > API break..?
> >
>
> I was thinking about that as well, but yea don't like the API break there,
> maybe we can add get_default and mark get_instance deprecated for now?

ok

> > > 2) it'd be nice to add `public bool contains (string id)`, to support 'in'
> > > operator.
> >
> > Also thought about that one. If we could overload it with a version that
> takes
> > an AppInfo then maybe, but this is too much of a Valaism to my liking. The
> lib
> > is meant to have a sensible C and Python API as well. If we can fix it with
> > some clever #defines or -custom.vala overrides then I am OK with it, but as
> a
> > part of the ABI I don't like it...
>
> Right, it'd be best to have that only in the vapi, unfortunately valac doesn't
> do -custom.vala when you're generating vapi from vala sources. The only thing
> we could do would be some sed :( Let's forget about that then...

We can look at it down the road if we find time for it

« Back to merge proposal