On 29.06.2015 12:50, Nikolay Sivov wrote:
On 29.06.2015 13:13, Sebastian Lackner wrote:
Thanks for the feedback. In this case pOutputPin is basically part of the same object, not an arbitrary external interface. It is constructed with VfwPin_Construct -> BaseOutputPin_Construct, and also holds a reference to the critical section of the parent object for example, so there is no clear distinction between public/private interfaces anyway.
Most other code parts which use BaseOutputPin_Construct store the result in something like: (IPin **)&This->store_impl_pointer_here where the variable already has the corresponding implementation type. Should we change it here too? Using only the public interface is not possible, take a look at for example the following code part, where a private member is set from a different interface:
--- snip --- This->driver_info = qcap_driver_init( This->pOutputPin, var.__VARIANT_NAME_1.__VARIANT_NAME_2.__VARIANT_NAME_3.ulVal ); if (This->driver_info) { pin = impl_from_IPin(This->pOutputPin); pin->driver_info = This->driver_info; // <--- --- snip ---
I personally prefer to use public interface, usually it makes things more clear, and code doesn't depend on implementation changes.
If in this particular case it's not possible then yes, we should store impl pointer instead of interface pointer.
Okay, I'll resend an updated version later then.
By the way, variant thing looks a little insane, assuming intention was to get V_UI4(&var), which is not necessary right on its own as qcap_driver_init() takes USHORT argument.
Yes, I also noticed it. Changes to this part should probably be in a separate patch though.