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]