GNOME Bugzilla – Bug 627397
Use better interface names
Last modified: 2011-01-31 20:26:10 UTC
The interfaces should have better names: * Alias -> Aliasable * Avatar -> AvatarOwner * Favourite -> Favouritable * Groups -> Groupable * Presence -> PresenceOwner I'm not too sure about AvatarOwner and PresenceOwner, but I couldn't come up with anything better.
(In reply to comment #0) > The interfaces should have better names: > * Alias -> Aliasable > * Avatar -> AvatarOwner > * Favourite -> Favouritable > * Groups -> Groupable > * Presence -> PresenceOwner > > I'm not too sure about AvatarOwner and PresenceOwner, but I couldn't come up > with anything better. I don't think "favouritable" is a word either, but that didn't stop you there :) For consistency, it might be best to just use avatarable and presencable/presenceable
(In reply to comment #1) > (In reply to comment #0) > > The interfaces should have better names: > > * Alias -> Aliasable > > * Avatar -> AvatarOwner > > * Favourite -> Favouritable > > * Groups -> Groupable > > * Presence -> PresenceOwner > > > > I'm not too sure about AvatarOwner and PresenceOwner, but I couldn't come up > > with anything better. > > I don't think "favouritable" is a word either, but that didn't stop you there > :) No, but I think its meaning is more recognisable than the meanings of "Avatarable" and "Presencable". "FavouriteOwner" wouldn't make sense. > For consistency, it might be best to just use avatarable and > presencable/presenceable Although consistency is good, using words which actually exist is probably better (easier to remember, type and understand). I'd vote for AvatarOwner and PresenceOwner over Avatarable and Precencable. Maybe this is something to ask about on #gnome-hackers. There's bound to be someone there who can come up with decent interface names.
I've taken the liberty of pushing changes to rename Alias → Aliasable and Groups → Groupable, since those are the two changes which aren't disputed. commit 4d343ba155404c07b700cd431e27973beeb1e7fb Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Mon Sep 13 14:28:56 2010 +0100 Rename Alias interface to Aliasable Helps: bgo#627397 backends/key-file/kf-persona.vala | 2 +- backends/telepathy/lib/tpf-persona.vala | 2 +- folks/Makefile.am | 2 +- folks/alias.vala | 36 ------------------------------- folks/aliasable.vala | 36 +++++++++++++++++++++++++++++++ folks/individual.vala | 18 +++++++------- tools/import-pidgin.vala | 4 +- 7 files changed, 50 insertions(+), 50 deletions(-) commit 74c267a0046f4dd067984def2456f25abc2f9d94 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Mon Sep 13 14:10:06 2010 +0100 Rename Groups interface to Groupable Helps: bgo#627397 backends/key-file/kf-persona-store.vala | 8 +- backends/telepathy/lib/tpf-persona-store.vala | 8 +- backends/telepathy/lib/tpf-persona.vala | 2 +- folks/Makefile.am | 2 +- folks/groupable.vala | 123 +++++++++++++++++++++++++ folks/groups.vala | 123 ------------------------- folks/individual-aggregator.vala | 4 +- folks/individual.vala | 26 +++--- folks/persona-store.vala | 2 +- tools/import-pidgin.vala | 4 +- 10 files changed, 151 insertions(+), 151 deletions(-)
(Mass changing milestones; please search for this phrase to delete the bug spam.)
Any further thoughts on these three renamings? • Avatar → AvatarOwner • Favourite → Favouritable • Presence → PresenceOwner Rob's suggested Avatar → HasAvatar, which could work for Presence as well.
(In reply to comment #5) > Any further thoughts on these three renamings? > • Avatar → AvatarOwner > • Favourite → Favouritable > • Presence → PresenceOwner > > Rob's suggested Avatar → HasAvatar, which could work for Presence as well. I'm fine with Favouritable, HasAvatar, and HasPresence.
Fixed! I'll make a release in the next few days, once the fix for bug #636714 has been reviewed and merged. commit 272e05c498430c2584753ce452e6ffd7fdbffbbe Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Sun Dec 12 18:32:51 2010 +0000 Rename the Favourite interface to Favouritable Closes: bgo#627397 NEWS | 2 + backends/telepathy/lib/tpf-persona.vala | 4 +- folks/Makefile.am | 2 +- folks/favouritable.vala | 33 +++++++++++++++++++++++++++++++ folks/favourite.vala | 33 ------------------------------- folks/individual.vala | 10 ++++---- 6 files changed, 43 insertions(+), 41 deletions(-) commit 5953792452846ee0df317c946a19522a77d15e1d Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Sun Dec 12 18:28:20 2010 +0000 Rename the Presence interface to HasPresence Helps: bgo#627397 NEWS | 1 + backends/telepathy/lib/tpf-persona.vala | 8 +- folks/Makefile.am | 2 +- folks/has-presence.vala | 161 ++++++++++++++++++++++++++++ folks/imable.vala | 2 +- folks/individual.vala | 11 +- folks/presence.vala | 161 ---------------------------- tests/telepathy/individual-properties.vala | 2 +- 8 files changed, 175 insertions(+), 173 deletions(-) commit 4c35a03bf5a6efe5ee3a865ba4d79857d57fc9c2 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Sun Dec 12 18:24:08 2010 +0000 Rename the Avatar interface to HasAvatar Helps: bgo#627397 NEWS | 3 ++ backends/telepathy/lib/tpf-persona.vala | 4 +- folks/Makefile.am | 2 +- folks/avatar.vala | 36 ------------------------------- folks/has-avatar.vala | 36 +++++++++++++++++++++++++++++++ folks/individual.vala | 6 ++-- 6 files changed, 45 insertions(+), 42 deletions(-)
Why not just using Avatar, Alias, Presence, etc.? The new names feel really artificial, they are quite long (fine in Vala, less fine when using them from C) and inconsistent.
(In reply to comment #8) > Why not just using Avatar, Alias, Presence, etc.? The new names feel really > artificial, The original names sounded like they were actually instances of the respective objects (e.g. Avatar sounds like an object which is an image, rather than an interface which has a method to get an avatar). > they are quite long (fine in Vala, less fine when using them from > C) and inconsistent. They should only be named FolksHasAvatar in C (vs. HasAvatar in Vala — 5 characters shorter), or have I got this wrong?
(In reply to comment #9) > (In reply to comment #8) > > Why not just using Avatar, Alias, Presence, etc.? The new names feel really > > artificial, > > The original names sounded like they were actually instances of the respective > objects (e.g. Avatar sounds like an object which is an image, rather than an > interface which has a method to get an avatar). But if you have “foo = FOLKS_HAS_AVATAR(individual)” what do you expect it to do? The name makes me think it returns a bool.
I'd like to second this: I think a macro called FOLKS_HAS_AVATAR() should return a boolean. I'd expect to do something like this: if (FOLKS_HAS_AVATAR (individual)) { FolksPortraiture *p = FOLKS_PORTRAITURE (individual); ... } but I think that, as things stand, FOLKS_HAS_AVATAR() will critical if the individual does not have one, since it's a checked cast, not just a check. Presumably the check is called FOLKS_IS_HAS_AVATAR()? Eugh. :)
(In reply to comment #11) > I'd like to second this: I think a macro called FOLKS_HAS_AVATAR() should > return a boolean. I'd expect to do something like this: > > if (FOLKS_HAS_AVATAR (individual)) > { > FolksPortraiture *p = FOLKS_PORTRAITURE (individual); > > ... > > } > > but I think that, as things stand, FOLKS_HAS_AVATAR() will critical if the > individual does not have one, since it's a checked cast, not just a check. > Presumably the check is called FOLKS_IS_HAS_AVATAR()? Eugh. :) Hmm, yeah, I didn't think of this when I reviewed this change. We should probably revert before the set the stage for capabilities macros like FOLKS_CAN_HAZ_AVATAR(). The original names (FolksAvatar, FolksPresence) weren't too terrible and generated reasonable C macros and functions. And as far as precedent, it looks like we wouldn't be the first one to use nouns for interfaces. Eg, Java's io class has interfaces yet only a few of them are adjectives: http://download.oracle.com/javase/6/docs/api/java/io/package-summary.html So it's probably worth a little naming inconsistency for less-insane generated API. Philip, thoughts?
(In reply to comment #12) > (In reply to comment #11) > > I'd like to second this: I think a macro called FOLKS_HAS_AVATAR() should > > return a boolean. I'd expect to do something like this: > > > > if (FOLKS_HAS_AVATAR (individual)) > > { > > FolksPortraiture *p = FOLKS_PORTRAITURE (individual); > > > > ... > > > > } > > > > but I think that, as things stand, FOLKS_HAS_AVATAR() will critical if the > > individual does not have one, since it's a checked cast, not just a check. > > Presumably the check is called FOLKS_IS_HAS_AVATAR()? Eugh. :) > > Hmm, yeah, I didn't think of this when I reviewed this change. We should > probably revert before the set the stage for capabilities macros like > FOLKS_CAN_HAZ_AVATAR(). I'm possibly more to blame. I made the corresponding changes in Empathy without noticing that I was typing something abominable. > The original names (FolksAvatar, FolksPresence) weren't too terrible and > generated reasonable C macros and functions. And as far as precedent, it looks > like we wouldn't be the first one to use nouns for interfaces. Eg, Java's io > class has interfaces yet only a few of them are adjectives: > http://download.oracle.com/javase/6/docs/api/java/io/package-summary.html > > So it's probably worth a little naming inconsistency for less-insane generated > API. As I see it, Java only uses nouns for interface names where they're interfaces only to allow several completely different implementations of the same object (e.g. the strategy design pattern[1]). For cases where interfaces are being used to “add on” functionality to a class, as here, it uses the “-able” adjective form. Even if nobody else thinks that's true, using Avatar and Alias for the interface names will come back to haunt us if we later want to add an actual class for avatar or alias instances (which is possible, though more likely in the former rather than the latter case). That leaves us with the option from before, or another one I just thought of. More suggestions are certainly welcome: • HasAvatar → AvatarOwner or AvatarInterface • HasPresence → PresenceOwner or PresenceInterface [1]: http://en.wikipedia.org/wiki/Strategy_pattern
(In reply to comment #13) > Even if nobody else thinks that's true, using Avatar and Alias for the gak. Make that “Avatar and Presence”.
(In reply to comment #13) > Even if nobody else thinks that's true, using Avatar and Alias for the > interface names will come back to haunt us if we later want to add an actual > class for avatar or alias instances (which is possible, though more likely in > the former rather than the latter case). It's already happening to me now. I'm defining a PostalAddress interface that has a “addresses” property that is list of structs. I would call this structure PostalAddress, except that this name is already used.
What about WithPresence, WithAvatar, WithAlias, WithIM etc?
(In reply to comment #16) > What about WithPresence, WithAvatar, WithAlias, WithIM etc? I think it's probably best to just rename HasAvatar and HasPresence. The other interfaces' names are OK. I can see WithAvatar and WithPresence working – FolksWithAvatar, Persona implements WithAvatar and FOLKS_IS_WITH_AVATAR() all sound OK – but something doesn't quite feel right about it. Then again, nothing feels right. Here's another suggestion which doesn't feel right: • HasAvatar → AvatarContainer • HasPresence → PresenceContainer
(In reply to comment #13) > That leaves us with the option from before, or another one I just thought of. > More suggestions are certainly welcome: > • HasAvatar → AvatarOwner or AvatarInterface > • HasPresence → PresenceOwner or PresenceInterface These options are my favorite. And I prefer Owner the most on the grounds that FOLKS_IS_AVATAR_OWNER (foo) makes slightly more sense than FOLKS_IS_AVATAR_INTERFACE (bar), since technically we're checking if bar is a FolksAvatarInterface implementation rather than the interface itself. Same in Vala for "is AvatarOwner" vs "is AvatarInterface". Container is similarly fine, though Owner is shorter, so I think it wins (and it sounds slightly better for some reason). I'll vote against With*, since FOLKS_IS_WITH_AVATAR() honestly looks odd to me. I keep wanting to edit it to FOLKS_HAS_AVATAR() or similar. Unless anyone has further objections, I say we just go with AvatarOwner, PresenceOwner and stick with the adjective/noun scheme Philip suggested in comment 13 (though I think we'll have to be careful to agree upon what future functionality is "added on" vs. core, since it seems a little subjective with our structure (Persona implementing nothing and Persona derivatives implementing all the supported interfaces)).
Created attachment 176913 [details] [review] Rename HasAvatar -> AvatarOwner, HasPresence -> PresenceOwner Squashed diff of branch: http://git.collabora.co.uk/?p=user/treitter/folks.git;a=shortlog;h=refs/heads/bgo627397-interface-names
Review of attachment 176913 [details] [review]: We might want to wait until just before making the next release to commit this, since we're going to have to make a release immediately after committing it for Empathy's benefit.
commit cea73b98512bb82a81b67fc1bbbf147543c8f046 Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Wed Dec 22 18:52:55 2010 -0800 Rename HasPresence -> PresenceOwner. Fixes bgo#627397. NEWS | 2 + backends/telepathy/lib/tpf-persona.vala | 6 +- folks/Makefile.am | 2 +- folks/has-presence.vala | 161 --------------------------- folks/individual.vala | 8 +- folks/presence-owner.vala | 162 ++++++++++++++++++++++++++++ tests/telepathy/individual-properties.vala | 2 +- 7 files changed, 173 insertions(+), 170 deletions(-) commit d80e62a4502e67971655bfefcaf91849654cc15d Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Wed Dec 22 18:07:15 2010 -0800 Rename HasAvatar -> AvatarOwner. Helps bgo#627397. NEWS | 3 ++ backends/telepathy/lib/tpf-persona.vala | 4 +- folks/Makefile.am | 2 +- folks/avatar-owner.vala | 36 +++++++++++++++++++++++++++++++ folks/has-avatar.vala | 36 ------------------------------- folks/individual.vala | 6 ++-- 6 files changed, 45 insertions(+), 42 deletions(-)