On 11/7/2012 01:05, Christian Costa wrote:

Le 06/11/2012 22:38, Michael Stefaniuc a écrit :

On 11/06/2012 08:51 PM, Christian Costa wrote:

Le 06/11/2012 20:26, Nikolay Sivov a écrit :

On 11/6/2012 20:47, Christian Costa wrote:
What I also meant is these names are redundant, something like
directmusicloader_AddRef() is enough, of course you could use mixed
casing in names if you want, there's no strict guidelines for that afaik.

I like to see the interface name in the function name because the method
alone does not mean anything.

Something like this:
<object name>Impl_<interface name>_<method name>
but in some case where there is only 1 interface for the object:
<interface name>Impl_<method name>
is better.

The interface name should be clearly visible IMO it's easier to read in
a log file.
IDirectMusicLoaderImpl_IDirectMusicLoader_AddRef

I disagree. It is redundant, too long and thus hard to read.
I even have patches that remove some of them. I've never submitted those
as you beat me to the dm* COM cleanup.

I agree too if there is only one interface. I think it is but I would like to be sure before submitting a patch.
I will check it and submit a patch for that.
Too long and hard to read, I don't agree. See my response to Nikolay.

Your example of having two methods with the same name in same object could be different things:

- two separate not inherited interfaces will have IUnknown part in common at least, in this case it doesn't make sense
  to have traces from all of them, you use "main" interface to implement IUnknown part and forward to it.

Having methods like
  <maininterface>_<another interface>_AddRef is strange when you can reduce that to <another inerface>_AddRef

- 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;

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.

For that particular case dmloader_ is enough to prefix names IMO.

If dmloader object has really 1 interface, I would rename then
IDirectMusicLoaderImpl_AddRef.
Although this is probably the case, I'm not sure yet. In all cases, I
would leave this for another patch. One thing at once.

Sure, separate patch to remove the duplicate name is fine. Just start
with that as it makes the subsequent COM cleanup patch shorter as it
cleans up the function header (whitespace and LPJUNK).

That's what I try to do altough I don't clearly understand what the problem with LPJUNK stuff.

Why would you use LPVOID, LPDWORD or LPCWSTR when it could be cleanly expressed without typedefs?

Also naming convention of having type-dependent prefix like dwLen or lpcszFileName is just a noise, why have 5 letters prefix that supposed to express variable type, I'll check type anyway, and lpcszFileName turns to filename with no info loss.