commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Thomas <>
Subject Re: [pool] Question / potential improvement on KeyedObjectPool
Date Mon, 30 Jul 2012 13:48:52 GMT
On 30/07/2012 09:36, Liviu Tudor wrote:
> Thanks very much for pointing me in the right direction, Mark!
> As with all development tasks, this, however, opened up new questions --
> not entirely sure if these are best discussed on this list or whether
> occasionally I should open up a JIRA ticket for this so more than
> welcoming suggestions here.

Jira is not the place for discussions. That is best done on the dev
list. Once we have an agreed direction then we can open a Jira to track
the work.

> * on the [pool2] -- I haven't investigated the eviction mechanism in
> detail there, but I understand the need for a List and that's fine. My
> original idea was to provide a Set with all the keys for the
> KeyedObjectPool interface, as there is no need for ordering of the keys
> here (same as in Map.keySet) however, I can just as well have a List and
> provide a getKeys() in the KeyedObjectPool interface -- it will be used
> the same ultimately for my idea, which in fact needs just a collection
> (Set was preferred due to the fact that Map provides access to the keys
> via a Set and the fact that keys are unique obviously). So, in the light
> of that, which would be better to have in KeyedObjectPool interface (if
> any):
>   1/ Set<K> keySet();	//returns a Set with all the keys in the pool
>   2/ List<K> getKeys(); //this is already implemented part of the Mbean
> implementation in GenericKeyedObjectPool
>   3/ Collection<K> getKeys();//this allows us to customise it in
> subclasses, so we can have List in GenericKeyedObjectPool but perhaps
> other implementation can return Set if needed

Since the keys are a Set, that seems like the most appropriate option.
I'd be happy changing the existing method to use Set. We should think
about switching the internal list as well, e.g. to a LinkedHashSet
(would need to check behaviour was unchanged).

> * Looking at [pool-1.6] -- there are a few problems with the code on that
> branch, not sure if this was abandoned or not?

Absolutely not.

> The layout of the project
> is a bit non-maven, but that is fine. Checkstyle throws a bunch of errors

Checkstyle 'errors' are not bugs. Cleaner code is nice but not essential.

> due to misconfiguration in pom.xml (which I have easily fixed). Also there
> are a bunch of FindBungs which I have fixed too.

Based on previous experience they may or may not be valid.

> There are a few unit
> tests failing which I am looking at while also trying to slot in my actual
> commit (as per below) :)

That sounds like your setup isn't quite right. The unit tests should
always pass and did on my machine the last time I touched the 1.x code.
There are also regular CI builds that check this.

> So, what are the best practices here -- separate these into a bunch of
> JIRA issues/patches :
>   1/ checkstyle fixes
>   2/ findbugs fixes
>   3/ unit test fixes
>   4/ my component
> Or just put everything into one patch?

One issue per Jira please.


To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message