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

Christian Costa titan.costa at gmail.com
Tue Nov 6 16:05:23 CST 2012


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.
>
>> 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.
I agree in case thare is only one interface and only one implementation.
I don't know what you mean by "The stuff at the moment is highly 
duplicated".
strmbase mostly exists because there is such a library in SDK.
>
>> 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.




More information about the wine-devel mailing list