[PATCH 1/3] dmloader: COM cleanup of IDirectMusicLoader object.

Michael Stefaniuc mstefani at redhat.com
Tue Nov 6 15:38:47 CST 2012


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.

> And for example if an object has 2 interfaces X and Y that both derives
> from Z then you will have 2 methods with the same name.
> How will you make the distinction ?
You don't need to make a distinction as you have in most of the cases
only one implementation. The stuff at the moment is highly duplicated.
Needs either to use a solution like strmbase where a base object is
included or COM aggregation. Didn't dig too deep into that as I stopped
my COM cleanup work for dm* for now.

> 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).

bye
	michael



More information about the wine-devel mailing list