Thanks a lot Seb, your suggestions are always appreciated :) Thanks for the sample, I see your troubles on ctor/builder, I think it makes much more sense when the number of arguments is very large, like the Generic(Keyed)ObjectPool(Factory). Do you have options to suggest? Thanks in advance.
About the reconfigure() method and config arguments re-arrangement, I'm going to apply the modifications you suggested. Simo http://people.apache.org/~simonetripodi/ http://www.99soft.org/ On Thu, Oct 28, 2010 at 8:10 PM, sebb <seb...@gmail.com> wrote: > 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 > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
