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

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

> 1) libunity is inconsistent in method names to get singleton instance
> (get_instance vs get_default), I'd rather see us using get_default which is
> more common in gobject world.

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.

But thinking about it again today I don't think API consumers is gonna appreciate this pedantry ;-) I'll rename it to get_default().

In the same spirit I think we should rename Unity.AppInfoManager.get_instance() as well - although that may be too hard an API break..?

> 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...

« Back to merge proposal