<br><br><div class="gmail_quote">2012/11/8 Michael Stefaniuc <span dir="ltr"><<a href="mailto:mstefani@redhat.com" target="_blank">mstefani@redhat.com</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On 11/07/2012 06:40 PM, Christian Costa wrote:<br>
> 2012/11/7 Michael Stefaniuc <<a href="mailto:mstefani@redhat.com">mstefani@redhat.com</a><br>
> <mailto:<a href="mailto:mstefani@redhat.com">mstefani@redhat.com</a>>><br>
<div class="im">> On 11/07/2012 02:50 PM, Christian Costa wrote:<br>
</div><div class="im">> > I didn't write this code and I don't like the current name either<br>
> but a<br>
> > method name does not mean anything if you don't know the interface it<br>
> > belongs to.<br>
> > And I would put the real interface name as for the method. It's a bit<br>
> > like renaming GetBufferDataPointer method to getbuf just because it's<br>
> > shorter.<br>
> Actually that's what Nikolay said. The function name needs to be<br>
> <interface>Impl_<method> where interface/method are the official names<br>
> from the API. What is superfluous/redundant information is to prepend<br>
> all that with the object name. Which in the case of dm* is the name of<br>
> the primary interface + "Impl".<br>
><br>
><br>
> So in this particular case: IDirectMusicLoaderImpl_AddRef, right ?<br>
</div>Right.<br>
<div><div class="h5"><br>
> > - object implements interface that overrides some of methods<br>
> from an<br>
> > interface it's inherited, this is a bit special and in dwrite for<br>
> > example it's done like<br>
> dwritetextlayout_layout___GetFontCollection()<br>
> > and dwritetextlayout___GetFontCollection(). But that is more an<br>
> > exception because of dwrite C++ nature;<br>
> ><br>
> ><br>
> > The method name is GetFontCollection but what is the interface ?<br>
> > ILayout, ITextLayout, ... I have to dig in the code and do some grep.<br>
> > And why don't you use getfontcollection instead, or gfntcollec?<br>
> ><br>
> ><br>
> > Why it's good to have short and clean names? Because that will get<br>
> > you clean traces where name doesn't eat half of line width.<br>
> ><br>
> ><br>
> > It seems "clean" is somewhat subjective. I prefer also short line<br>
> in log<br>
> > but this meaningfull name.<br>
> > Just take quartz with object that can have up to 10 interfaces and<br>
> > interfaces that can be implemented by up to 10 objects.<br>
> > Put a bit of multithreading on top of it (and I don't mention the fact<br>
> > the TRACE are not serialized between thread).<br>
> I the case of quartz most of the duplicated interfaces are implemented<br>
> by the same base (C++) object. Or should be implemented that way. So<br>
> while multiple COM classes support the same interface most of the time<br>
> there is one implementation only.<br>
><br>
><br>
> It's partly true. Sometimes some methods are overridden.<br>
><br>
><br>
><br>
><br>
> If not the next question to ask is if it really makes sense to have two<br>
> monster COM classes in the same .c file.<br>
><br>
><br>
> Yes of course but you will have the same function name in the debugger and<br>
> in the trace.<br>
</div></div>I'll pass on the debugger as I'm not a debugger person.<br>
<br>
TRACE on the other hand is simple. Sometimes there is a different debug<br>
channel in use. Or the trace message itself can contain the info. But I<br>
also made the experience that more often than not one has to follow the<br>
interface/object pointer from the TRACE to be able to distinguish<br>
between different instances of the same COM class. I mean follow it back<br>
to the creation/initialization of the object which gives the needed info<br>
to find the right implementation.<br></blockquote><div><br></div><div>Relying on that just because we use functions with the same name is awkward imo. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
> For example in dmusic/port.c there is 3 port objects for Synth,<br>
> MidiOut and MidiIn each one supported 3 interfaces<br>
> IDirectMusicPort, IDirectMusicPassThru<br>
> and possibly IDirectMusicPortDownload. Altough it is ok to use for<br>
> example IDirectMusicPortImpl_<Method><br>
> for the common methods implementation but you will need to specify the<br>
> object name<br>
> for the methods that are specific to each port. Unless you keep a<br>
> generic implementation and use<br>
</div>I've seen different techniques in use, it really depends on how big the<br>
implementations differ. It might be even solvable with an if-else<br>
if (This->has_ds8) // if is_primary(This)<br>
foo;<br>
else<br>
bar;<br>
<br>
Other times there is a different vtable with different degrees of<br>
methods in common.<br></blockquote><div><br></div><div>I know these techniques and all are good. The best one to use depends</div><div>on the situation.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
> a functions table as it is done sometimes in strmbase but is less C++ like.<br>
</div>You make it sound like "less C++ like" would be bad. Quite the contrary,<br>
not being bound by the C++ object model gives you flexibility. There was<br>
a great LWN article on the object oriented programming/design patterns<br>
in the Linux Kernel, it is worth a read for anybody interested in object<br>
oriented programming in C.<br></blockquote><div><br></div><div>Well no. Was just a comment. I'm not a pro C++.</div><div>That said naming conventions should not be a constraint to choose a particular</div><div>technique over another one.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
> Of course Alexandre might not be convinced about a file split but in<br>
> that case to avoid functions with the same name use something short to<br>
> prefix it. I prefer using the upper letters of the COM class as a<br>
> prefix, e.g.: dsb_IUnknown_QueryInterface as opposed to<br>
> IDirectSoundBuffer8Impl_IUnknown_QueryInterface. By the time I get to<br>
> the method name I have forgotten the starting part of the function name.<br>
> Also I don't get confused by seeing two interface names in the same<br>
> function name.<br>
><br>
> Personally I would have used :<br>
> dsb_IUnknown_QueryInterface<br>
> dsb_IDirectSoundBuffer8_QueryInterface<br>
> dsb_Ixxxx_yyyy<br>
><br>
> So we have object_interface_method. This is more formal and clear and we<br>
> can remove the "Impl" suffix.<br>
> Only one simple rule and if we keep the object name short the function<br>
> name is not too long.<br>
</div>We did think about using the object name, but there is no COM object<br>
naming standard in Wine and some of the object names could be better.<br>
Using object_interface_method could yield stuff like<br>
"struct_IFooBarImpl_IFooBar_QueryInterface()" which isn't a beauty.<br></blockquote><div><br></div><div>Well why struct_I*Impl ?</div><div>It seems there is a confusion between class and interface.</div><div>
For example CLSID_DirectMusicLoader is the class and IDirectMusicLoader is the interface.</div><div>Off course there are tightly tied together because CLSID_DirectMusicLoader has only 1 interface and</div><div>IDirectMusicLoader is only implement by this class.</div>
<div><br></div><div>In that case I would just use _IDirectMusicLoader_QueryInterface() (with an underscore prefix)</div><div>That would work also for an generic interface whose implementation is shared by several classes.</div>
<div><br></div><div>And if we have to make a class distinction we just add the class name so we have dmload_IDirectMusicLoader_QueryInterface.</div><div><br></div><div>So we have the pattern (class)_interface_method() which work in every situation and enable homogeneity.</div>
<div><br></div><div>This concept of primary interface is a bit odd imo. Saying that IBaseFilter is the primary interface of the video renderer does not</div><div>make sense.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br><span class="HOEnZb"><font color="#888888"><br>
</font></span></blockquote></div><br>