On 28 October 2010 18:50, Simone Tripodi <simone.trip...@gmail.com> wrote:
> Hi Seb,
> thanks for your feedbacks. Please read my questions inline your comments
>
>> The public StackObjectPoolConfig ctor repeats the settings available
>> in the nested Builder.
>> I'm not sure I see the point of having both. Or perhaps the ctor is
>> supposed to be private?
>
> I'm sorry but I didn't understand. Can you provide me a short sample
> please? Many thanks in advance :P

StackObjectPoolConfig config1 = new StackObjectPoolConfig(maxSleep,initIdle)

creates an instance which is equal to the one created by

StackObjectPoolConfig config2 = new StackObjectPoolConfig.Builder()
        .setInitIdleCapacity(initIdle)
        .setMaxSleeping(maxSleep)
        .createConfig();

>
>> Also, I don't like the way the ctor resets the parameters if they are
>> out of range, especially as this is not documented.
>>
>
> As user, I don't too, but I took inspiration from the old 1.5 code.
> Take a look at http://s.apache.org/tS

Yes, but that does at least document the parameter munging.

>
>> ==
>>
>> The StackObjectPool ctor calls the overridable method
>> reconfigure(StackObjectPoolConfig) which is not safe when subclassing.
>> Easily fixed by having a private method with the shared code.
>>
>
> I suppose the idea was to expose that method to allow users
> reconfigure the pool just passing the whole configuration and not
> setting the parameters one by one. So I suggest to make `final
> synchronized` that method. Thoughts?

Making the method final is another option; needs to be documented why
it was done.

>
>> Also, reconfigure() updates maxSleeping, but is not synchronized, so
>> may not publish maxSleeping correctly.
>>
>> Similar considerations apply to the other classes.
>
> Thanks a lot, have a nice day!
> Simo
>
>>
>>> If this fits to our vision, I can follow applying the refactor to
>>> Generic(Keyed)ObjectPool(Factory).
>>> Many thanks in advance!!!
>>> All the best,
>>> Simo
>>>
>>> [1] http://svn.apache.org/viewvc?rev=1028302&view=rev
>>>
>>> http://people.apache.org/~simonetripodi/
>>> http://www.99soft.org/
>>>
>>>
>>>
>>> On Thu, Oct 28, 2010 at 2:46 PM, Simone Tripodi
>>> <simone.trip...@gmail.com> wrote:
>>>> Hi James,
>>>> thanks, understood. I take in charge yet another cycle of refactoring,
>>>> I'll ping you all when in trouble about something :)
>>>> Have a nice day and thanks for the feedbacks!
>>>> Simo
>>>>
>>>> http://people.apache.org/~simonetripodi/
>>>> http://www.99soft.org/
>>>>
>>>>
>>>>
>>>> On Thu, Oct 28, 2010 at 2:12 PM, James Carman
>>>> <ja...@carmanconsulting.com> wrote:
>>>>> On Thu, Oct 28, 2010 at 6:50 AM, Simone Tripodi
>>>>> <simone.trip...@gmail.com> wrote:
>>>>>> That's why I wouldn't follow the option #2; but please, explain me
>>>>>> which are the side effects of that design, so I can avoid to repeat
>>>>>> the same mistake in the future.
>>>>>>
>>>>>
>>>>> If you make the config objects immutable, you can just keep the config
>>>>> references around and skip the copying in the reconfigure() method.  I
>>>>> don't think it would be difficult to implement things this way.  We
>>>>> don't want to let the JMX stuff dictate our API too much.  The JMX
>>>>> stuff can be less "elegant" for sure.  Consider it to be just a UI
>>>>> layer on top of our stuff.
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>>>>> For additional commands, e-mail: dev-h...@commons.apache.org
>>>>>
>>>>>
>>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>>> For additional commands, e-mail: dev-h...@commons.apache.org
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>> For additional commands, e-mail: dev-h...@commons.apache.org
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to