On 11/14/05, robert burrell donkin <[EMAIL PROTECTED]> wrote:
> On Fri, 2005-11-11 at 02:32 -0500, Sandy McArthur wrote:
> > * The javadacs for [Keyed]ObjectPool.borrowObject should list
> > NoSuchElementException as one of the thrown exceptions. Techically a
> > NSEE is included with "throws Exception" but listing NSEE would help
> > clients better deal with exceptions. If you are using a pool that
> > limits the number of active objects then a NSEE isn't that big of a
> > deal relative to any other exception which would indicate a truely
> > unexpected problem. Since NoSuchElementException extends
> > RuntimeException adding this wouldn't break API compatability.
>
> +1
>
> IMHO throwing exception is reasonable but really the API needs to be
> helping the client code understand the exceptions thrown. (personally
> speaking i prefer checked exceptions but they come with their own
> issues.)

I'm not suggesting the throws Exception is removed, just that
NoSuchElementException is added to the documentation.

> > * [Keyed]PoolableObjectFactory in the package description asserts that
> > activateObject will be invoked on every object returned from the pool.
> > The method description asserts that activeObject will be used to
> > "reinitialize an instance to be returned by the pool." What is
> > ambiguous is if activateObject is used to new created objects from
> > makeObject.
> >
> > It's my opinion that activateObject should only be used on existing
> > objects that are being returned from the internal idle object pool.
> > This behavior has the advantage that it's possibly more efficient. In
> > my experience newly made objects are almost always in the same state
> > that an activated object would be in.
>
> +1
>
> IMHO there will be some use cases where calling activateObject on the
> constructed object is more elegant. however, there seems little point in
> demanding this behaviour generally. a decorating factory calling
> activateObject on the product would allow any 1.x factories which
> require this to be adapted.

If a PoolableObjectFactory implementation needs to call activateObject
on newly created objects there is no reason it cannot make that call
from within the makeObject method.

> > * The [Keyed]PoolableObjectFactory getNumIdle and getNumActive methods
> > currently throw an UnsupportedOperationException if they aren't
> > supported. I kinda think it would be easier on clients if they simply
> > reuturned a negative value when those operations are not supported. As
> > it is now if you have code similar to:
> > if (pool.getNumActive() > 15) { log.message("borrowed more than expected"); 
> > }
> > you really need to wrap it in a try/catch to be safe which I find a
> > bit silly. Since UnsupportedOperationException is a RuntimeException
> > removing this doesn't break API compatability.
>
> this is a question of style and i've heard both sides of this argument.
> however, i do suspect that the exception handling for pool is rather an
> arcane art and over complex. there may be an argument for allowing
> implementations flexibility as to whether they thrown an exception or
> return a negative value.

I don't strongly care one way or another. But please do not allow
both. Pick one.

--
Sandy McArthur

"He who dares not offend cannot be honest."
- Thomas Paine

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to