On 11/07/2012 06:40 PM, Christian Costa wrote:
> 2012/11/7 Michael Stefaniuc <mstefani@redhat.com
> <mailto:mstefani@redhat.com>>

>     On 11/07/2012 02:50 PM, Christian Costa wrote:

>     > I didn't write this code and I don't like the current name either
>     but a
>     > method name does not mean anything if you don't know the interface it
>     > belongs to.
>     > And I would put the real interface name as for the method. It's a bit
>     > like renaming GetBufferDataPointer method to getbuf just because it's
>     > shorter.
>     Actually that's what Nikolay said. The function name needs to be
>     <interface>Impl_<method> where interface/method are the official names
>     from the API. What is superfluous/redundant information is to prepend
>     all that with the object name. Which in the case of dm* is the name of
>     the primary interface + "Impl".
>
>
> So in this particular case: IDirectMusicLoaderImpl_AddRef, right ?

Right.

>     >     - object implements interface that overrides some of methods
>     from an
>     >     interface it's inherited, this is a bit special and in dwrite for
>     >     example it's done like
>     dwritetextlayout_layout___GetFontCollection()
>     >     and dwritetextlayout___GetFontCollection(). But that is more an
>     >     exception because of dwrite C++ nature;
>     >
>     >
>     > The method name is GetFontCollection but what is the interface ?
>     > ILayout, ITextLayout, ... I have to dig in the code and do some grep.
>     > And why don't you use getfontcollection instead, or gfntcollec?
>     >
>     >
>     >     Why it's good to have short and clean names? Because that will get
>     >     you clean traces where name doesn't eat half of line width.
>     >
>     >
>     > It seems "clean" is somewhat subjective. I prefer also short line
>     in log
>     > but this meaningfull name.
>     > Just take quartz with object that can have up to 10 interfaces and
>     > interfaces that can be implemented by up to 10 objects.
>     > Put a bit of multithreading on top of it (and I don't mention the fact
>     > the TRACE are not serialized between thread).
>     I the case of quartz most of the duplicated interfaces are implemented
>     by the same base (C++) object. Or should be implemented that way. So
>     while multiple COM classes support the same interface most of the time
>     there is one implementation only.
>
>
> It's partly true. Sometimes some methods are overridden.
>
>
>
>
>     If not the next question to ask is if it really makes sense to have two
>     monster COM classes in the same .c file.
>
>
> Yes of course but you will have the same function name in the debugger and
> in the trace.

I'll pass on the debugger as I'm not a debugger person.

TRACE on the other hand is simple. Sometimes there is a different debug
channel in use. Or the trace message itself can contain the info. But I
also made the experience that more often than not one has to follow the
interface/object pointer from the TRACE to be able to distinguish
between different instances of the same COM class. I mean follow it back
to the creation/initialization of the object which gives the needed info
to find the right implementation.

> For example in dmusic/port.c there is 3 port objects for Synth,
> MidiOut and MidiIn each one supported 3 interfaces
> IDirectMusicPort, IDirectMusicPassThru
> and possibly IDirectMusicPortDownload. Altough it is ok to use for
> example IDirectMusicPortImpl_<Method>
> for the common methods implementation but you will need to specify the
> object name
> for the methods that are specific to each port. Unless you keep a
> generic implementation and use

I've seen different techniques in use, it really depends on how big the
implementations differ. It might be even solvable with an if-else
    if (This->has_ds8) // if is_primary(This)
        foo;
    else
        bar;

Other times there is a different vtable with different degrees of
methods in common.

> a functions table as it is done sometimes in strmbase but is less C++ like.

You make it sound like "less C++ like" would be bad. Quite the contrary,
not being bound by the C++ object model gives you flexibility. There was
a great LWN article on the object oriented programming/design patterns
in the Linux Kernel, it is worth a read for anybody interested in object
oriented programming in C.

>     Of course Alexandre might not be convinced about a file split but in
>     that case to avoid functions with the same name use something short to
>     prefix it. I prefer using the upper letters of the COM class as a
>     prefix, e.g.: dsb_IUnknown_QueryInterface as opposed to
>     IDirectSoundBuffer8Impl_IUnknown_QueryInterface. By the time I get to
>     the method name I have forgotten the starting part of the function name.
>     Also I don't get confused by seeing two interface names in the same
>     function name.
>
> Personally I would have used :
> dsb_IUnknown_QueryInterface
> dsb_IDirectSoundBuffer8_QueryInterface
> dsb_Ixxxx_yyyy
>
> So we have object_interface_method. This is more formal and clear and we
> can remove the "Impl" suffix.
> Only one simple rule and if we keep the object name short the function
> name is not too long.

We did think about using the object name, but there is no COM object
naming standard in Wine and some of the object names could be better.
Using object_interface_method could yield stuff like
"struct_IFooBarImpl_IFooBar_QueryInterface()" which isn't a beauty.