Accept Iterable rather than Collection everywhere we reasonably can.

Note one potentially consequential diff in DepotRepository:

-        return keys.isEmpty() ? Collections.<T>emptyList() :
-            _ctx.invoke(new FindAllQuery.WithKeys<T>(_ctx, keys));
+        return _ctx.invoke(new FindAllQuery.WithKeys<T>(_ctx, keys));

This should not actually impact anything. FindAllQuery.WithKeys first checks
for keys that are cached, coping with a potentially empty set (there will be
none), then if there are keys that remain to be loaded from the database, it
loads them. The first check will NOOP as will the second check. So this should
just be a slightly more circuitous path to getting back an empty collection. It
should not result in a zero row matching query being sent to the database.

With that in mind, it seemed inelegant to prematurely optmize the return of an
empty collection.
This commit is contained in:
Michael Bayne
2010-02-25 06:48:27 +00:00
parent 950653960b
commit b8d957ce4c
9 changed files with 28 additions and 28 deletions
@@ -32,7 +32,7 @@ import java.util.Map;
import java.util.Set;
import com.google.common.base.Function;
import com.google.common.collect.Collections2;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
@@ -182,12 +182,12 @@ public abstract class DepotRepository
* @throws DatabaseException if any problem is encountered communicating with the database.
*/
public <T extends PersistentRecord> List<T> loadAll (
Class<T> type, Collection<? extends Comparable<?>> primaryKeys)
Class<T> type, Iterable<? extends Comparable<?>> primaryKeys)
throws DatabaseException
{
// convert the raw keys into real key records
return loadAll(
Collections2.transform(primaryKeys, _ctx.getMarshaller(type).primaryKeyFunction()));
Iterables.transform(primaryKeys, _ctx.getMarshaller(type).primaryKeyFunction()));
}
/**
@@ -195,11 +195,10 @@ public abstract class DepotRepository
*
* @throws DatabaseException if any problem is encountered communicating with the database.
*/
public <T extends PersistentRecord> List<T> loadAll (Collection<Key<T>> keys)
public <T extends PersistentRecord> List<T> loadAll (Iterable<Key<T>> keys)
throws DatabaseException
{
return keys.isEmpty() ? Collections.<T>emptyList() :
_ctx.invoke(new FindAllQuery.WithKeys<T>(_ctx, keys));
return _ctx.invoke(new FindAllQuery.WithKeys<T>(_ctx, keys));
}
/**
@@ -227,14 +226,14 @@ public abstract class DepotRepository
* @throws DatabaseException if any problem is encountered communicating with the database.
*/
public <T extends PersistentRecord> List<T> findAll (
Class<T> type, Collection<? extends QueryClause> clauses)
Class<T> type, Iterable<? extends QueryClause> clauses)
throws DatabaseException
{
return findAll(type, CacheStrategy.BEST, clauses);
}
/**
* A varargs version of {@link #findAll(Class,CacheStrategy,Collection)}.
* A varargs version of {@link #findAll(Class,CacheStrategy,Iterable)}.
*
* @throws DatabaseException if any problem is encountered communicating with the database.
*/
@@ -251,7 +250,7 @@ public abstract class DepotRepository
* @throws DatabaseException if any problem is encountered communicating with the database.
*/
public <T extends PersistentRecord> List<T> findAll (
Class<T> type, CacheStrategy cache, Collection<? extends QueryClause> clauses)
Class<T> type, CacheStrategy cache, Iterable<? extends QueryClause> clauses)
throws DatabaseException
{
DepotMarshaller<T> marsh = _ctx.getMarshaller(type);
@@ -331,7 +330,7 @@ public abstract class DepotRepository
* @throws DatabaseException if any problem is encountered communicating with the database.
*/
public <T extends PersistentRecord> List<Key<T>> findAllKeys (
Class<T> type, boolean forUpdate, Collection<? extends QueryClause> clauses)
Class<T> type, boolean forUpdate, Iterable<? extends QueryClause> clauses)
throws DatabaseException
{
return _ctx.invoke(new FindAllKeysQuery<T>(_ctx, type, forUpdate, clauses));
+5 -5
View File
@@ -20,7 +20,7 @@
package com.samskivert.depot;
import java.util.Collection;
import com.google.common.collect.Iterables;
import com.samskivert.depot.clause.SelectClause;
import com.samskivert.depot.expression.FluentExp;
@@ -49,9 +49,9 @@ public class Ops
/**
* Creates an AND expression with the supplied target expressions.
*/
public static FluentExp and (Collection<? extends SQLExpression> conditions)
public static FluentExp and (Iterable<? extends SQLExpression> conditions)
{
return and(conditions.toArray(new SQLExpression[conditions.size()]));
return and(Iterables.toArray(conditions, SQLExpression.class));
}
/**
@@ -86,9 +86,9 @@ public class Ops
/**
* Creates an OR expression with the supplied target expressions.
*/
public static FluentExp or (Collection<? extends SQLExpression> conditions)
public static FluentExp or (Iterable<? extends SQLExpression> conditions)
{
return or(conditions.toArray(new SQLExpression[conditions.size()]));
return or(Iterables.toArray(conditions, SQLExpression.class));
}
/**
@@ -1,3 +1,6 @@
//
// $Id$
package com.samskivert.depot;
import java.util.Collection;
@@ -53,7 +53,7 @@ public class SelectClause
* class for rows that match the supplied clauses.
*/
public SelectClause (Class<? extends PersistentRecord> pClass, ColumnExp[] columns,
Collection<? extends QueryClause> clauses)
Iterable<? extends QueryClause> clauses)
{
_pClass = pClass;
_fields = columns;
@@ -55,7 +55,7 @@ public class ColumnExp extends FluentExp
}
/** Returns an {@link In} with this column and the supplied values. */
public In in (Collection<? extends Comparable<?>> values)
public In in (Iterable<? extends Comparable<?>> values)
{
return new In(this, values);
}
@@ -23,7 +23,6 @@ package com.samskivert.depot.impl;
import java.sql.SQLException;
import java.util.Arrays;
import java.util.Collection;
import java.util.Map;
import java.util.Set;
@@ -58,7 +57,7 @@ public class DepotTypes
* are interrogated for their class definition sets through {@link SQLExpression#addClasses}.
*/
public static <T extends PersistentRecord> DepotTypes getDepotTypes (
PersistenceContext ctx, Collection<? extends QueryClause> clauses)
PersistenceContext ctx, Iterable<? extends QueryClause> clauses)
throws DatabaseException
{
Set<Class<? extends PersistentRecord>> classSet = Sets.newLinkedHashSet();
@@ -71,7 +70,7 @@ public class DepotTypes
}
/**
* A varargs version of {@link #getDepotTypes(PersistenceContext,Collection)}.
* A varargs version of {@link #getDepotTypes(PersistenceContext,Iterable)}.
*/
public static <T extends PersistentRecord> DepotTypes getDepotTypes (
PersistenceContext ctx, QueryClause... clauses)
@@ -84,7 +83,7 @@ public class DepotTypes
* Create a new DepotTypes with the given {@link PersistenceContext} and a collection of
* persistent record classes.
*/
public DepotTypes (PersistenceContext ctx, Collection<Class<? extends PersistentRecord>> others)
public DepotTypes (PersistenceContext ctx, Iterable<Class<? extends PersistentRecord>> others)
throws DatabaseException
{
for (Class<? extends PersistentRecord> c : others) {
@@ -21,7 +21,6 @@
package com.samskivert.depot.impl;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.sql.Connection;
@@ -46,7 +45,7 @@ import static com.samskivert.depot.Log.log;
public class FindAllKeysQuery<T extends PersistentRecord> extends Query<List<Key<T>>>
{
public FindAllKeysQuery (PersistenceContext ctx, Class<T> type, boolean forUpdate,
Collection<? extends QueryClause> clauses)
Iterable<? extends QueryClause> clauses)
throws DatabaseException
{
_forUpdate = forUpdate;
@@ -26,7 +26,6 @@ import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
@@ -66,7 +65,7 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
public static class WithCache<T extends PersistentRecord> extends FindAllQuery<T>
{
public WithCache (PersistenceContext ctx, Class<T> type,
Collection<? extends QueryClause> clauses, CacheStrategy strategy)
Iterable<? extends QueryClause> clauses, CacheStrategy strategy)
throws DatabaseException
{
super(ctx, type);
@@ -201,7 +200,7 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
public static class Explicitly<T extends PersistentRecord> extends FindAllQuery<T>
{
public Explicitly (PersistenceContext ctx, Class<T> type,
Collection<? extends QueryClause> clauses, boolean cachedContents)
Iterable<? extends QueryClause> clauses, boolean cachedContents)
throws DatabaseException
{
super(ctx, type);
@@ -22,6 +22,7 @@ package com.samskivert.depot.impl.operator;
import java.util.Collection;
import com.google.common.collect.Iterables;
import com.samskivert.depot.PersistentRecord;
import com.samskivert.depot.expression.SQLExpression;
import com.samskivert.depot.impl.FragmentVisitor;
@@ -41,9 +42,9 @@ public class In
_values = values;
}
public In (SQLExpression pColumn, Collection<? extends Comparable<?>> values)
public In (SQLExpression pColumn, Iterable<? extends Comparable<?>> values)
{
this(pColumn, values.toArray(new Comparable<?>[values.size()]));
this(pColumn, Iterables.toArray(values, Comparable.class));
}
public SQLExpression getExpression ()