From b8d957ce4cb8658e338fb1fa14fa43931f102c67 Mon Sep 17 00:00:00 2001 From: Michael Bayne Date: Thu, 25 Feb 2010 06:48:27 +0000 Subject: [PATCH] Accept Iterable rather than Collection everywhere we reasonably can. Note one potentially consequential diff in DepotRepository: - return keys.isEmpty() ? Collections.emptyList() : - _ctx.invoke(new FindAllQuery.WithKeys(_ctx, keys)); + return _ctx.invoke(new FindAllQuery.WithKeys(_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. --- .../com/samskivert/depot/DepotRepository.java | 19 +++++++++---------- src/java/com/samskivert/depot/Ops.java | 10 +++++----- .../com/samskivert/depot/SQLFragment.java | 3 +++ .../samskivert/depot/clause/SelectClause.java | 2 +- .../depot/expression/ColumnExp.java | 2 +- .../com/samskivert/depot/impl/DepotTypes.java | 7 +++---- .../depot/impl/FindAllKeysQuery.java | 3 +-- .../samskivert/depot/impl/FindAllQuery.java | 5 ++--- .../samskivert/depot/impl/operator/In.java | 5 +++-- 9 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/java/com/samskivert/depot/DepotRepository.java b/src/java/com/samskivert/depot/DepotRepository.java index 9013973..3bb6e26 100644 --- a/src/java/com/samskivert/depot/DepotRepository.java +++ b/src/java/com/samskivert/depot/DepotRepository.java @@ -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 List loadAll ( - Class type, Collection> primaryKeys) + Class type, Iterable> 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 List loadAll (Collection> keys) + public List loadAll (Iterable> keys) throws DatabaseException { - return keys.isEmpty() ? Collections.emptyList() : - _ctx.invoke(new FindAllQuery.WithKeys(_ctx, keys)); + return _ctx.invoke(new FindAllQuery.WithKeys(_ctx, keys)); } /** @@ -227,14 +226,14 @@ public abstract class DepotRepository * @throws DatabaseException if any problem is encountered communicating with the database. */ public List findAll ( - Class type, Collection clauses) + Class type, Iterable 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 List findAll ( - Class type, CacheStrategy cache, Collection clauses) + Class type, CacheStrategy cache, Iterable clauses) throws DatabaseException { DepotMarshaller marsh = _ctx.getMarshaller(type); @@ -331,7 +330,7 @@ public abstract class DepotRepository * @throws DatabaseException if any problem is encountered communicating with the database. */ public List> findAllKeys ( - Class type, boolean forUpdate, Collection clauses) + Class type, boolean forUpdate, Iterable clauses) throws DatabaseException { return _ctx.invoke(new FindAllKeysQuery(_ctx, type, forUpdate, clauses)); diff --git a/src/java/com/samskivert/depot/Ops.java b/src/java/com/samskivert/depot/Ops.java index 90cd187..114febc 100644 --- a/src/java/com/samskivert/depot/Ops.java +++ b/src/java/com/samskivert/depot/Ops.java @@ -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 conditions) + public static FluentExp and (Iterable 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 conditions) + public static FluentExp or (Iterable conditions) { - return or(conditions.toArray(new SQLExpression[conditions.size()])); + return or(Iterables.toArray(conditions, SQLExpression.class)); } /** diff --git a/src/java/com/samskivert/depot/SQLFragment.java b/src/java/com/samskivert/depot/SQLFragment.java index 02a2989..9b3c56f 100644 --- a/src/java/com/samskivert/depot/SQLFragment.java +++ b/src/java/com/samskivert/depot/SQLFragment.java @@ -1,3 +1,6 @@ +// +// $Id$ + package com.samskivert.depot; import java.util.Collection; diff --git a/src/java/com/samskivert/depot/clause/SelectClause.java b/src/java/com/samskivert/depot/clause/SelectClause.java index 7fef802..0891763 100644 --- a/src/java/com/samskivert/depot/clause/SelectClause.java +++ b/src/java/com/samskivert/depot/clause/SelectClause.java @@ -53,7 +53,7 @@ public class SelectClause * class for rows that match the supplied clauses. */ public SelectClause (Class pClass, ColumnExp[] columns, - Collection clauses) + Iterable clauses) { _pClass = pClass; _fields = columns; diff --git a/src/java/com/samskivert/depot/expression/ColumnExp.java b/src/java/com/samskivert/depot/expression/ColumnExp.java index 0346477..560ae5d 100644 --- a/src/java/com/samskivert/depot/expression/ColumnExp.java +++ b/src/java/com/samskivert/depot/expression/ColumnExp.java @@ -55,7 +55,7 @@ public class ColumnExp extends FluentExp } /** Returns an {@link In} with this column and the supplied values. */ - public In in (Collection> values) + public In in (Iterable> values) { return new In(this, values); } diff --git a/src/java/com/samskivert/depot/impl/DepotTypes.java b/src/java/com/samskivert/depot/impl/DepotTypes.java index c650acc..7e48090 100644 --- a/src/java/com/samskivert/depot/impl/DepotTypes.java +++ b/src/java/com/samskivert/depot/impl/DepotTypes.java @@ -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 DepotTypes getDepotTypes ( - PersistenceContext ctx, Collection clauses) + PersistenceContext ctx, Iterable clauses) throws DatabaseException { Set> 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 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> others) + public DepotTypes (PersistenceContext ctx, Iterable> others) throws DatabaseException { for (Class c : others) { diff --git a/src/java/com/samskivert/depot/impl/FindAllKeysQuery.java b/src/java/com/samskivert/depot/impl/FindAllKeysQuery.java index c64cf42..ae34965 100644 --- a/src/java/com/samskivert/depot/impl/FindAllKeysQuery.java +++ b/src/java/com/samskivert/depot/impl/FindAllKeysQuery.java @@ -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 extends Query>> { public FindAllKeysQuery (PersistenceContext ctx, Class type, boolean forUpdate, - Collection clauses) + Iterable clauses) throws DatabaseException { _forUpdate = forUpdate; diff --git a/src/java/com/samskivert/depot/impl/FindAllQuery.java b/src/java/com/samskivert/depot/impl/FindAllQuery.java index d968c2c..d840c99 100644 --- a/src/java/com/samskivert/depot/impl/FindAllQuery.java +++ b/src/java/com/samskivert/depot/impl/FindAllQuery.java @@ -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 extends Query extends FindAllQuery { public WithCache (PersistenceContext ctx, Class type, - Collection clauses, CacheStrategy strategy) + Iterable clauses, CacheStrategy strategy) throws DatabaseException { super(ctx, type); @@ -201,7 +200,7 @@ public abstract class FindAllQuery extends Query extends FindAllQuery { public Explicitly (PersistenceContext ctx, Class type, - Collection clauses, boolean cachedContents) + Iterable clauses, boolean cachedContents) throws DatabaseException { super(ctx, type); diff --git a/src/java/com/samskivert/depot/impl/operator/In.java b/src/java/com/samskivert/depot/impl/operator/In.java index 474ea82..756b140 100644 --- a/src/java/com/samskivert/depot/impl/operator/In.java +++ b/src/java/com/samskivert/depot/impl/operator/In.java @@ -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> values) + public In (SQLExpression pColumn, Iterable> values) { - this(pColumn, values.toArray(new Comparable[values.size()])); + this(pColumn, Iterables.toArray(values, Comparable.class)); } public SQLExpression getExpression ()