[Bug 38142] proxy factories

[bugzilla-daemon at freedesktop.org]

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

--- Comment #4 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2011-06-10 03:02:20 PDT ---
(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.

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

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

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 

> 1) create TpSimpleClientFactory, subclass of GObject, that will have virtual
> methods like _create_channel(); _create_connection();
> _dup_features_for_channel(); with default implementation to return
> tp_channel_new(); tp_connection_new(). It will also have a TpProxy
> *tp_simple_client_factory_proxy_lookup(self, object_path); that gives the
> cached proxy if already exists.

This could just be TpBasicProxyFactory, probably.

> Technically that would be a 'protected' method
> in C++ terms used by subclasses that reimplement some methods to see if a proxy
> already exists, but doesn't hurt to be public.

protected and public are very similar, in practice - they're part of your API
and ABI (because library consumers can call them).

> 3) TpUserClientFactory, subclass of TpAutomaticClientFactory, that will
> reimplement _dup_features_for_connection() by first chaining up to parent impl
> to get the features from the automatic factory, then add features asked by user
> using something like tp_user_client_factory_add_features_for_connections().

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.

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

> 4) EmpathyClientFactory, subclass of TpUserClientFactory, that will do
> reimplement methods for creating EmpathyTpChat and EmpathyContact subclasses,
> but relies on parent implementation for all the rest.

This probably does want to be a subclass.

> This proposal does not need interfaces, all is virtual methods.

Which is good because?

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