[Bug 38142] proxy factories

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=38142

--- Comment #6 from Xavier Claessens <xclaesse at gmail.com> 2011-06-10 03:50:57 PDT ---
(In reply to comment #4)
> (In reply to comment #1)
> > What I don't like with current code:
> > 
> > 1) TpBasicProxyFactory is supposed to _implement_
> > TpClientChannelFactoryInterface, but it actually have no code at all, since the
> > interface already implement the methods. I think the interface should print
> > warning and return NULL if one of its method is called but not actually
> > implemented by the object.
> 
> That's an ABI break; previously-working code can stop working.
> 
> In general, a new virtual method added to an interface or base class can have
> either of these semantics:
> 
> * Method is optional, interface/base class does something
>   vaguely sensible (typically "do nothing" or "raise GError", but in
>   this case "instantiate basic object") if it wasn't provided, callers of
>   the new virtual method don't need to check before calling it
> 
> * Method is optional, callers of the method must check (somehow) before
>   calling it and not call it if it's unimplemented/unsuitable, which means
>   there must be some way to check whether it's implemented or not
>   (capability flags, maybe)
> 
> It can't be mandatory without breaking the interface's ABI.

Right, I know that would be ABI break. That's one (small) reason why my
proposal does not reuse those API but add completely new and deprecate the
rest.

> > IMO TpAutomaticProxyFactory should be SUBCLASS of
> > TpBasicProxyFactory
> 
> ABI break, unless sizeof (TpBasicProxyFactory) == sizeof
> (TpAutomaticProxyFactory.parent) (i.e. no priv pointer), in which case it's OK.
> 
> Happily, TpBasicProxyFactory has no priv pointer, so this rearrangement would
> be an option.

I know that's an ABI break, just like above, that's another reason to create
new API instead of reusing this.

> > 3) struct _TpBasicProxyFactory {GObject parent;}; --> no place to add a
> > "gpointer priv" without breaking ABI. But we need to add a priv struct for
> > holding the proxy cache for uniqueness...
> 
> You can do private access the slow way, without a cache (priv): just call
> G_OBJECT_INSTANCE_GET_PRIVATE_DATA (or whatever the macro is called) every time
> you need to get it.

as above: yet another reason to deprecate those API and add new one instead of
trying to extend it and be annoyed by ABI constrains.

> The advantage of using a GInterface (as opposed to an abstract or non-abstract
> base class) for, well, interfaces, is that you can multiple-inherit from
> interfaces but you can't multiple-inherit from objects. An object can implement
> ClientChannelFactory and ClientConnectionFactory simultaneously.
> 
> If we use a base class for factories, that base class has to know about every
> flavour of of proxy that there will ever be. (It can 

TpBaseClientFactory surely knows about all flavors of TpProxy. If app (empathy)
create its own, it can subclass TpBaseClientFactory to add its own proxy
construct (with empathy_factory_create_foo), and all other proxies will still
be implemented as well, no need to reimplement the interfaces.

The big reason I don't like the interface way, is that you can't easilly
reimplement an interface method in your subclass but still chainup to parent's
implementation of that same interface. Or is that possible/easy/nice with
GObject??

> > 3) TpUserClientFactory, subclass of TpAutomaticClientFactory
> 
> If the user is going to set their desired features via methods, there's no need
> for a subclass - just fold this into the Automatic (or even Basic) class. The
> user will have to create their own instance, call methods on it, then pass it
> around (instead of relying on a singleton-generating function like
> tp_dup_default_proxy_factory() or something), but they'd have to do that
> anyway.

I like that! So in my proposal: remove TpUserClientFactory and add
tp_simple_client_factory_add_*_features(). That makes even more important for
factory subclasses to chainup to parent implementation to get the user-set
features and yet add the special features of EmpathyTpChat for example.

> If the user is going to set their desired features via subclassing then they
> might as well subclass the Automatic or Basic class directly.

While technically that's an option, in practice doing GObject subclasses is so
boring I would like to avoid apps doing it. Unless really necessary, like
temporally for EmpathyTpChat. Also defining desired features by subclassing
factory means we won't be able to add features at run-time.

> > This proposal does not need interfaces, all is virtual methods.
> 
> Which is good because?

Said above: chaining to parent implementation to "extend" instead of "replace"
the behavior of methods. But maybe there is something I don't know in
GInterface that makes this possible using interfaces?

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.