DO NOT REPLY [Bug 33477] - [dbutils] Stored Procedure Runner and Bean Reuse Handler code submission

[bu...@apache.org]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=33477>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=33477





------- Additional Comments From dgraham@apache.org  2005-07-29 01:34 -------
This bug is a bit confusing because it addresses two unrelated items.

BeanReuseHandler:
What is the use case for reusing an object rather than just creating a new one?

The implementation of this class is rather strange and seems likely to lead to
bugs.  Why is there a queue of objects to reuse?  Why is the first object that
happens to match the instanceof check populated and not others?


StoredProcRunner:
The reason there hasn't been demand for this support is that stored procedures
are RDBMS specific so are generally avoided like the plague.  Regardless, the
public API and implementation of this class is not easy to understand.  In no
small part due to the missing javadoc.

It forces you to pass in a Connection rather than retrieve one from the
QueryRunner's DataSource which is a better practice than manual Connection
handling.  

The getOutParameterValues() method creates an ArrayList and converts to an
Object[] when it could simply create the array to begin with.  

There are two loop indexes, j and i, in fillStatementOutParameters() when one
would do.

There are unusual constructs like:
new Object[] {} when a simple new Object[0] would work.  

The null checking in if statements is backwards.  This is a common idiom in C
but you will receive a compile error in Java if you try:
if (myvar = null) so there's no point in forcing people to read the statement
backwards.  

You can only get access to the out parameters.  What about the ResultSet?

The brace positioning is inconsistent some if statements use the correct
if (true) {
  // do something
}
form while other for loops and methods use the incorrect:
for ()
{
  // do something
}.

For these reasons, I am against adding this code to DbUtils in its current form.

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

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