<br><br><div class="gmail_quote">2012/11/7 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 02:50 PM, Christian Costa wrote:<br>
><br>
><br>
> 2012/11/7 Nikolay Sivov <<a href="mailto:bunglehead@gmail.com">bunglehead@gmail.com</a> <mailto:<a href="mailto:bunglehead@gmail.com">bunglehead@gmail.com</a>>><br>
<div class="im">><br>
> On 11/7/2012 01:05, Christian Costa wrote:<br>
><br>
> Le 06/11/2012 22:38, Michael Stefaniuc a écrit :<br>
><br>
> On 11/06/2012 08:51 PM, Christian Costa wrote:<br>
><br>
> Le 06/11/2012 20:26, Nikolay Sivov a écrit :<br>
><br>
> On 11/6/2012 20:47, Christian Costa wrote:<br>
> What I also meant is these names are redundant,<br>
> something like<br>
> directmusicloader_AddRef() is enough, of course you<br>
> could use mixed<br>
> casing in names if you want, there's no strict<br>
> guidelines for that afaik.<br>
><br>
> I like to see the interface name in the function name<br>
> because the method<br>
> alone does not mean anything.<br>
><br>
> Something like this:<br>
> <object name>Impl_<interface name>_<method name><br>
> but in some case where there is only 1 interface for the<br>
> object:<br>
> <interface name>Impl_<method name><br>
> is better.<br>
><br>
> The interface name should be clearly visible IMO it's<br>
> easier to read in<br>
> a log file.<br>
</div>> IDirectMusicLoaderImpl___IDirectMusicLoader_AddRef<br>
<div><div class="h5">><br>
> I disagree. It is redundant, too long and thus hard to read.<br>
> I even have patches that remove some of them. I've never<br>
> submitted those<br>
> as you beat me to the dm* COM cleanup.<br>
><br>
> I agree too if there is only one interface. I think it is but I<br>
> would like to be sure before submitting a patch.<br>
> I will check it and submit a patch for that.<br>
> Too long and hard to read, I don't agree. See my response to<br>
> Nikolay.<br>
><br>
> Your example of having two methods with the same name in same object<br>
> could be different things:<br>
><br>
> - two separate not inherited interfaces will have IUnknown part in<br>
> common at least, in this case it doesn't make sense<br>
> to have traces from all of them, you use "main" interface to<br>
> implement IUnknown part and forward to it.<br>
><br>
><br>
> I just use an example to show that we can have methods with the same<br>
> name. This other cases too off course.<br>
><br>
> Having methods like<br>
> <maininterface>_<another interface>_AddRef is strange when you can<br>
> reduce that to <another inerface>_AddRef<br>
><br>
><br>
> I didn't write this code and I don't like the current name either 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>
</div></div>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></blockquote><div><br></div><div>So in this particular case: IDirectMusicLoaderImpl_AddRef, right ?</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>
> - object implements interface that overrides some of methods from an<br>
> interface it's inherited, this is a bit special and in dwrite for<br>
</div>> example it's done like dwritetextlayout_layout___GetFontCollection()<br>
> and dwritetextlayout___GetFontCollection(). But that is more an<br>
<div class="im">> 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 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>
</div>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.</blockquote><div><br></div><div>It's partly true. Sometimes some methods are overridden.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
</blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<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></blockquote><div><br></div><div>Yes of course but you will have the same function name in the debugger and</div><div>in the trace.</div><div>For example in dmusic/port.c there is 3 port objects for Synth,</div>
<div>MidiOut and MidiIn each one supported 3 interfaces IDirectMusicPort, IDirectMusicPassThru</div><div>and possibly IDirectMusicPortDownload. Altough it is ok to use for example IDirectMusicPortImpl_<Method></div>
<div>for the common methods implementation but you will need to specify the object name</div><div>for the methods that are specific to each port. Unless you keep a generic implementation and use</div><div>a functions table as it is done sometimes in strmbase but is less C++ like. </div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<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></blockquote><div>Personally I would have used :</div><div>dsb_IUnknown_QueryInterface</div><div>dsb_IDirectSoundBuffer8_QueryInterface</div><div>dsb_Ixxxx_yyyy</div><div><br></div><div>So we have object_interface_method. This is more formal and clear and we can remove the "Impl" suffix.</div>
<div>Only one simple rule and if we keep the object name short the function name is not too long. </div><div><br></div><div>Christian</div></div><br>