You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@empire-db.apache.org by Francis De Brabandere <fr...@gmail.com> on 2009/07/06 23:14:35 UTC
Re: svn commit: r791597 - /incubator/empire-db/trunk/empire-db/src/main/java/org/apache/empire/db/DBReader.java
public final <T> ArrayList<T> getBeanList(Class<T> t, int maxItems)
The javadoc for this method does not indicate negative maxItems values
as no limit. And I think it would be better to say any negative value
instead of -1 since using other negative values is not documented at
the moment...
(for the generic method as well of course)
One more thing:
I believe it is bad practice to reuse parameters in your methods. It
is not expected that maxCount actually changes during execution. In my
eyes this is a micro-optimization that hinders readability.
So far my bitching :-)
On Mon, Jul 6, 2009 at 10:03 PM, <do...@apache.org> wrote:
> Author: doebele
> Date: Mon Jul 6 20:03:21 2009
> New Revision: 791597
>
> URL: http://svn.apache.org/viewvc?rev=791597&view=rev
> Log:
> EMPIREDB-42
>
> Modified:
> incubator/empire-db/trunk/empire-db/src/main/java/org/apache/empire/db/DBReader.java
>
> Modified: incubator/empire-db/trunk/empire-db/src/main/java/org/apache/empire/db/DBReader.java
> URL: http://svn.apache.org/viewvc/incubator/empire-db/trunk/empire-db/src/main/java/org/apache/empire/db/DBReader.java?rev=791597&r1=791596&r2=791597&view=diff
> ==============================================================================
> --- incubator/empire-db/trunk/empire-db/src/main/java/org/apache/empire/db/DBReader.java (original)
> +++ incubator/empire-db/trunk/empire-db/src/main/java/org/apache/empire/db/DBReader.java Mon Jul 6 20:03:21 2009
> @@ -24,6 +24,7 @@
> import java.sql.ResultSet;
> import java.sql.SQLException;
> import java.util.ArrayList;
> +import java.util.Collection;
> import java.util.HashMap;
> import java.util.Iterator;
> import java.util.Map;
> @@ -623,17 +624,17 @@
> }
>
> /**
> - * Returns the result of a query as a list of objects resticted
> + * Returns the result of a query as a list of objects restricted
> * to a maximum number of objects (unless maxCount is -1).
> *
> - * @param c the class type of the objects in the list
> + * @param c the collection to add the objects to
> + * @param t the class type of the objects in the list
> * @param maxCount the maximum number of objects
> - * @param <T> the type of the objects in the list
> *
> * @return the list of <T>
> */
> @SuppressWarnings("unchecked")
> - public <T> ArrayList<T> getBeanList(Class<T> c, int maxCount)
> + public <C extends Collection<T>, T> C getBeanList(C c, Class<T> t, int maxCount)
> {
> // Check Recordset
> if (rset == null)
> @@ -649,11 +650,10 @@
> for (int i = 0; i < colList.length; i++)
> paramTypes[i] = DBExpr.getValueClass(colList[i].getDataType());
> // Find Constructor
> - Constructor ctor = findMatchingAccessibleConstructor(c, paramTypes);
> + Constructor ctor = findMatchingAccessibleConstructor(t, paramTypes);
> Object[] args = (ctor!=null) ? new Object[getFieldCount()] : null;
>
> // Create a list of beans
> - ArrayList<T> list = new ArrayList<T>();
> while (moveNext() && maxCount != 0)
> { // Create bean an init
> if (ctor!=null)
> @@ -661,21 +661,21 @@
> for (int i = 0; i < getFieldCount(); i++)
> args[i] = getValue(i);
> T bean = (T)ctor.newInstance(args);
> - list.add(bean);
> + c.add(bean);
> }
> else
> { // Use Property Setters
> - T bean = c.newInstance();
> + T bean = t.newInstance();
> if (getBeanProperties(bean)==false)
> return null;
> - list.add(bean);
> + c.add(bean);
> }
> // Decrease count
> if (maxCount > 0)
> maxCount--;
> }
> // done
> - return list;
> + return c;
> } catch (InvocationTargetException e)
> {
> error(e);
> @@ -690,20 +690,30 @@
> return null;
> }
> }
> -
> +
> /**
> * Returns the result of a query as a list of objects.
> *
> - * @param c the class type of the objects in the list
> - * @param <T> the type of the objects in the list
> + * @param t the class type of the objects in the list
> + * @param maxCount the maximum number of objects
> *
> * @return the list of <T>
> */
> - public <T> ArrayList<T> getBeanList(Class<T> c)
> - {
> - return getBeanList(c, -1);
> + public final <T> ArrayList<T> getBeanList(Class<T> t, int maxItems) {
> + return getBeanList(new ArrayList<T>(), t, maxItems);
> }
> -
> +
> + /**
> + * Returns the result of a query as a list of objects.
> + *
> + * @param t the class type of the objects in the list
> + *
> + * @return the list of <T>
> + */
> + public final <T> ArrayList<T> getBeanList(Class<T> t) {
> + return getBeanList(t, -1);
> + }
> +
> /**
> * Moves the cursor down one row from its current position.
> *
>
>
>
--
http://www.somatik.be
Microsoft gives you windows, Linux gives you the whole house.
re: svn commit: r791597 - /incubator/empire-db/trunk/empire-db/src/main/java/org/apache/empire/db/DBReader.java
Posted by Rainer Döbele <do...@esteam.de>.
Hi Francis,
thanks for your comment and of course you are right.
The javadoc should be adjusted and I actually haven't rethought the reuse of maxItems since it was already in there - and I don't really want to touch this now.
Rainer
Francis De Brabandere wrote:
> Re: svn commit: r791597 - /incubator/empire-db/trunk/empire-
> db/src/main/java/org/apache/empire/db/DBReader.java
>
> public final <T> ArrayList<T> getBeanList(Class<T> t, int maxItems)
>
> The javadoc for this method does not indicate negative maxItems values
> as no limit. And I think it would be better to say any negative value
> instead of -1 since using other negative values is not documented at
> the moment...
> (for the generic method as well of course)
>
> One more thing:
> I believe it is bad practice to reuse parameters in your methods. It
> is not expected that maxCount actually changes during execution. In my
> eyes this is a micro-optimization that hinders readability.
> So far my bitching :-)
>
> On Mon, Jul 6, 2009 at 10:03 PM, <do...@apache.org> wrote:
> > Author: doebele
> > Date: Mon Jul 6 20:03:21 2009
> > New Revision: 791597
> >
> > URL: http://svn.apache.org/viewvc?rev=791597&view=rev
> > Log:
> > EMPIREDB-42
> >
> > Modified:
> > incubator/empire-db/trunk/empire-
> db/src/main/java/org/apache/empire/db/DBReader.java
> >
> > Modified: incubator/empire-db/trunk/empire-
> db/src/main/java/org/apache/empire/db/DBReader.java
> > URL: http://svn.apache.org/viewvc/incubator/empire-db/trunk/empire-
> db/src/main/java/org/apache/empire/db/DBReader.java?rev=791597&r1=79159
> 6&r2=791597&view=diff
> >
> =======================================================================
> =======
> > --- incubator/empire-db/trunk/empire-
> db/src/main/java/org/apache/empire/db/DBReader.java (original)
> > +++ incubator/empire-db/trunk/empire-
> db/src/main/java/org/apache/empire/db/DBReader.java Mon Jul 6 20:03:21
> 2009
> > @@ -24,6 +24,7 @@
> > import java.sql.ResultSet;
> > import java.sql.SQLException;
> > import java.util.ArrayList;
> > +import java.util.Collection;
> > import java.util.HashMap;
> > import java.util.Iterator;
> > import java.util.Map;
> > @@ -623,17 +624,17 @@
> > }
> >
> > /**
> > - * Returns the result of a query as a list of objects resticted
> > + * Returns the result of a query as a list of objects restricted
> > * to a maximum number of objects (unless maxCount is -1).
> > *
> > - * @param c the class type of the objects in the list
> > + * @param c the collection to add the objects to
> > + * @param t the class type of the objects in the list
> > * @param maxCount the maximum number of objects
> > - * @param <T> the type of the objects in the list
> > *
> > * @return the list of <T>
> > */
> > @SuppressWarnings("unchecked")
> > - public <T> ArrayList<T> getBeanList(Class<T> c, int maxCount)
> > + public <C extends Collection<T>, T> C getBeanList(C c, Class<T>
> t, int maxCount)
> > {
> > // Check Recordset
> > if (rset == null)
> > @@ -649,11 +650,10 @@
> > for (int i = 0; i < colList.length; i++)
> > paramTypes[i] =
> DBExpr.getValueClass(colList[i].getDataType());
> > // Find Constructor
> > - Constructor ctor = findMatchingAccessibleConstructor(c,
> paramTypes);
> > + Constructor ctor = findMatchingAccessibleConstructor(t,
> paramTypes);
> > Object[] args = (ctor!=null) ? new
> Object[getFieldCount()] : null;
> >
> > // Create a list of beans
> > - ArrayList<T> list = new ArrayList<T>();
> > while (moveNext() && maxCount != 0)
> > { // Create bean an init
> > if (ctor!=null)
> > @@ -661,21 +661,21 @@
> > for (int i = 0; i < getFieldCount(); i++)
> > args[i] = getValue(i);
> > T bean = (T)ctor.newInstance(args);
> > - list.add(bean);
> > + c.add(bean);
> > }
> > else
> > { // Use Property Setters
> > - T bean = c.newInstance();
> > + T bean = t.newInstance();
> > if (getBeanProperties(bean)==false)
> > return null;
> > - list.add(bean);
> > + c.add(bean);
> > }
> > // Decrease count
> > if (maxCount > 0)
> > maxCount--;
> > }
> > // done
> > - return list;
> > + return c;
> > } catch (InvocationTargetException e)
> > {
> > error(e);
> > @@ -690,20 +690,30 @@
> > return null;
> > }
> > }
> > -
> > +
> > /**
> > * Returns the result of a query as a list of objects.
> > *
> > - * @param c the class type of the objects in the list
> > - * @param <T> the type of the objects in the list
> > + * @param t the class type of the objects in the list
> > + * @param maxCount the maximum number of objects
> > *
> > * @return the list of <T>
> > */
> > - public <T> ArrayList<T> getBeanList(Class<T> c)
> > - {
> > - return getBeanList(c, -1);
> > + public final <T> ArrayList<T> getBeanList(Class<T> t, int
> maxItems) {
> > + return getBeanList(new ArrayList<T>(), t, maxItems);
> > }
> > -
> > +
> > + /**
> > + * Returns the result of a query as a list of objects.
> > + *
> > + * @param t the class type of the objects in the list
> > + *
> > + * @return the list of <T>
> > + */
> > + public final <T> ArrayList<T> getBeanList(Class<T> t) {
> > + return getBeanList(t, -1);
> > + }
> > +
> > /**
> > * Moves the cursor down one row from its current position.
> > *
> >
> >
> >
>
>
>
> --
> http://www.somatik.be
> Microsoft gives you windows, Linux gives you the whole house.