[vlc-devel] commit: Added picture_pool_NewExtended. (Laurent Aimar )

[Laurent Aimar]

Hi,
On Thu, Sep 10, 2009, Pierre d'Herbemont wrote:
>
> On Sep 8, 2009, at 9:18 AM, Laurent Aimar wrote:
>
>> On Tue, Sep 08, 2009, Pierre d'Herbemont wrote:
>>>
>>>
>>> On Sep 7, 2009, at 11:05 PM, "Rémi Denis-Courmont" <remi at remlab.net>
>>> wrote:
>>>
>>>> Le lundi 7 septembre 2009 23:57:52 Pierre d'Herbemont, vous avez  
>>>> écrit
>>>> :
>>>>> In this case NewWithConfiguration seems more appropriate. And
>>>>> "configuration" should  probably be "callback".
>>>>
>>>> Do you really need to invent the longest possible names all the  
>>>> time?
>>>
>>> You are just saying, that short is good. And I do agree.
>>>
>>> However someone new to the code will appreciate meaningful and clear
>>> name.   I am always open to better suggestion.
>>>
>>> Also, if you want to put it that way, Extended is a waste of 8 chars.
>>>
>>> I am concerned about this "lazy" way to extend existing API. And it
>>> shouldn't be considered as a good practice, even if it's a common  
>>> one.
>>> Will you end up with an ExtendedExtended if you want a more  
>>> specialized
>>> call?
>> No, picture_pool_configuration_t simply need to be extended.
>
> Err, this looks weird. What if it has nothing to do with the pool?
 Err, then it would not be a picture_pool_* function...

> I am just saying that this is not the best naming scheme ever. I think  
> we should avoid it in future API.
I want 2 constructors that will create a picture_pool_t object:
 1. one simple used for most cases.
 2. one complexe when full or not common control over the picture_pool
 properties are needed.

I decided to name 1. picture_pool_New and 2. picture_pool_NewExtended.
I also designed picture_pool_NewExtended in such a way that adding new
properties will required the minimal amount of code changes and no new
functions.

What's wrong with that?

>>> Generally the extended and non extend should be merged, or just  
>>> renamed.
>> I prefer to have 2 functions:
>> - one that will allow to be extended easily. picture_pool_NewExtended 
>> will do,
>> it just a matter of extending picture_pool_configuration_t.
>> - one that is easy to use (and it is the common use case),  
>> picture_pool_New.
>
> Probably, but why don't you use WithPool for the name or something more 
> descriptive than just extended.
 I don't understand you...

-- 
fenrir