2012/11/8 Michael Stefaniuc <[email protected]> > On 11/07/2012 06:40 PM, Christian Costa wrote: > > 2012/11/7 Michael Stefaniuc <[email protected] > > <mailto:[email protected]>> > > 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. >
Relying on that just because we use functions with the same name is awkward imo. > > > 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. > I know these techniques and all are good. The best one to use depends on the situation. > > > 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. > Well no. Was just a comment. I'm not a pro C++. That said naming conventions should not be a constraint to choose a particular technique over another one. > > > 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. > Well why struct_I*Impl ? It seems there is a confusion between class and interface. For example CLSID_DirectMusicLoader is the class and IDirectMusicLoader is the interface. Off course there are tightly tied together because CLSID_DirectMusicLoader has only 1 interface and IDirectMusicLoader is only implement by this class. In that case I would just use _IDirectMusicLoader_QueryInterface() (with an underscore prefix) That would work also for an generic interface whose implementation is shared by several classes. And if we have to make a class distinction we just add the class name so we have dmload_IDirectMusicLoader_QueryInterface. So we have the pattern (class)_interface_method() which work in every situation and enable homogeneity. This concept of primary interface is a bit odd imo. Saying that IBaseFilter is the primary interface of the video renderer does not make sense. > > >
