From 963a9a0fc1be4610450c0d595025047221d5a94d Mon Sep 17 00:00:00 2001 From: Michael Bayne Date: Wed, 8 Dec 2010 19:35:50 +0000 Subject: [PATCH] Guava makes defensive programming more pleasant. --- .../com/samskivert/depot/EHCacheAdapter.java | 12 ++--- src/main/java/com/samskivert/depot/Key.java | 36 +++++-------- .../com/samskivert/depot/QueryBuilder.java | 51 +++++++------------ .../com/samskivert/depot/SchemaMigration.java | 30 +++++------ .../samskivert/depot/clause/SelectClause.java | 34 ++++--------- .../samskivert/depot/clause/WhereClause.java | 10 ++-- 6 files changed, 65 insertions(+), 108 deletions(-) diff --git a/src/main/java/com/samskivert/depot/EHCacheAdapter.java b/src/main/java/com/samskivert/depot/EHCacheAdapter.java index ff5ebe0..bcd7408 100644 --- a/src/main/java/com/samskivert/depot/EHCacheAdapter.java +++ b/src/main/java/com/samskivert/depot/EHCacheAdapter.java @@ -39,6 +39,8 @@ import net.sf.ehcache.distribution.RMICacheReplicatorFactory; import net.sf.ehcache.event.CacheEventListener; import net.sf.ehcache.store.MemoryStoreEvictionPolicy; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; import static com.samskivert.depot.Log.log; /** @@ -149,9 +151,8 @@ public class EHCacheAdapter { long now = System.currentTimeMillis(); Ehcache cache = _categories.get(category); - if (cache == null) { - throw new IllegalArgumentException("Unknown category: " + category); - } + checkArgument(cache != null, "Unknown category: " + category); + @SuppressWarnings("unchecked") EHCacheBin bin = (EHCacheBin) _bins.get(cacheId); if (bin == null) { @@ -305,9 +306,8 @@ public class EHCacheAdapter { public EHCacheKey (String id, Serializable key) { - if (id == null || key == null) { - throw new IllegalArgumentException("Can't handle null key or id"); - } + checkNotNull(id); + checkNotNull(key); _id = id; _key = key; } diff --git a/src/main/java/com/samskivert/depot/Key.java b/src/main/java/com/samskivert/depot/Key.java index 780e995..957f2fa 100644 --- a/src/main/java/com/samskivert/depot/Key.java +++ b/src/main/java/com/samskivert/depot/Key.java @@ -37,6 +37,8 @@ import com.samskivert.depot.impl.DepotMarshaller; import com.samskivert.depot.impl.DepotUtil; import com.samskivert.depot.impl.FragmentVisitor; +import static com.google.common.base.Preconditions.checkArgument; + /** * A special form of {@link WhereClause} that uniquely specifies a single database row and thus * also a single persistent object. It knows how to invalidate itself upon modification. This class @@ -181,12 +183,9 @@ public class Key extends WhereClause // from ValidatingCacheInvalidator public void validateFlushType (Class pClass) { - if (!pClass.equals(_pClass)) { - throw new IllegalArgumentException( - "Class mismatch between persistent record and cache invalidator " + - "[record=" + pClass.getSimpleName() + - ", invtype=" + _pClass.getSimpleName() + "]."); - } + checkArgument(pClass.equals(_pClass), + "Class mismatch between persistent record and cache invalidator " + + "[record=%s, invtype=%s].", pClass.getSimpleName(), _pClass.getSimpleName()); } // from CacheInvalidator @@ -247,9 +246,8 @@ public class Key extends WhereClause protected static Comparable[] toCanonicalOrder ( Class pClass, ColumnExp[] fields, Comparable[] values) { - if (fields.length != values.length) { - throw new IllegalArgumentException("Field and Value arrays must be of equal length."); - } + checkArgument(fields.length == values.length, + "Field and Value arrays must be of equal length."); // look up the cached primary key fields for this object ColumnExp[] keyFields = DepotUtil.getKeyFields(pClass); @@ -269,24 +267,16 @@ public class Key extends WhereClause Comparable[] cvalues = new Comparable[values.length]; for (int ii = 0; ii < keyFields.length; ii++) { Comparable value = map.remove(keyFields[ii]); - if (value == null) { - // make sure we were provided with a value for this primary key field - throw new IllegalArgumentException( - "Missing value for key field: " + keyFields[ii]); - } - if (!(value instanceof Serializable)) { - throw new IllegalArgumentException( - "Non-serializable argument [key=" + keyFields[ii] + - ", value=" + value + "]"); - } + // make sure we were provided with a value for this primary key field + checkArgument(value != null, "Missing value for key field: " + keyFields[ii]); + checkArgument(value instanceof Serializable, + "Non-serializable argument [key=%s, value=%s]", keyFields[ii], value); cvalues[ii] = value; } // finally make sure we were not given any fields that are not primary key fields - if (map.size() > 0) { - throw new IllegalArgumentException( - "Non-key columns given: " + StringUtil.join(map.keySet().toArray(), ", ")); - } + checkArgument(map.isEmpty(), "Non-key columns given: " + + StringUtil.join(map.keySet().toArray())); return cvalues; } diff --git a/src/main/java/com/samskivert/depot/QueryBuilder.java b/src/main/java/com/samskivert/depot/QueryBuilder.java index 5e630e2..26599c5 100644 --- a/src/main/java/com/samskivert/depot/QueryBuilder.java +++ b/src/main/java/com/samskivert/depot/QueryBuilder.java @@ -32,6 +32,9 @@ import com.samskivert.depot.expression.SQLExpression; import com.samskivert.depot.impl.ColumnSet; import com.samskivert.depot.impl.FindAllQuery; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; + /** * The root of a fluent mechanism for constructing queries. Obtain an instance via {@link * DepotRepository#from}. @@ -96,9 +99,7 @@ public class QueryBuilder public QueryBuilder where (Iterable exprs) { Iterator iter = exprs.iterator(); - if (!iter.hasNext()) { - throw new IllegalArgumentException("Must supply at least one expression."); - } + checkArgument(iter.hasNext(), "Must supply at least one expression."); SQLExpression first = iter.next(); return where(iter.hasNext() ? new Where(Ops.and(exprs)) : new Where(first)); } @@ -128,7 +129,7 @@ public class QueryBuilder */ public QueryBuilder where (WhereClause where) { - requireNull(_where, "Where clause is already configured."); + checkState(_where == null, "Where clause is already configured."); _where = where; return this; } @@ -177,7 +178,7 @@ public class QueryBuilder */ public QueryBuilder groupBy (SQLExpression... exprs) { - requireNull(_groupBy, "GroupBy clause is already configured."); + checkState(_groupBy == null, "GroupBy clause is already configured."); _groupBy = new GroupBy(exprs); return this; } @@ -211,7 +212,7 @@ public class QueryBuilder */ public QueryBuilder orderBy (OrderBy orderBy) { - requireNull(_orderBy, "OrderBy clause is already configured."); + checkState(_orderBy == null, "OrderBy clause is already configured."); _orderBy = orderBy; return this; } @@ -221,7 +222,7 @@ public class QueryBuilder */ public QueryBuilder limit (int count) { - requireNull(_limit, "Limit clause is already configured."); + checkState(_limit == null, "Limit clause is already configured."); _limit = new Limit(0, count); return this; } @@ -231,7 +232,7 @@ public class QueryBuilder */ public QueryBuilder limit (int offset, int count) { - requireNull(_limit, "Limit clause is already configured."); + checkState(_limit == null, "Limit clause is already configured."); _limit = new Limit(offset, count); return this; } @@ -258,7 +259,7 @@ public class QueryBuilder */ public QueryBuilder override (FromOverride fromOverride) { - requireNull(_fromOverride, "FromOverride clause is already configured."); + checkState(_fromOverride == null, "FromOverride clause is already configured."); _fromOverride = fromOverride; return this; } @@ -304,7 +305,7 @@ public class QueryBuilder */ public QueryBuilder forUpdate () { - requireNull(_forUpdate, "ForUpdate clause is already configured."); + checkState(_forUpdate == null, "ForUpdate clause is already configured."); _forUpdate = new ForUpdate(); return this; } @@ -450,30 +451,16 @@ public class QueryBuilder } } - protected void requireNotNull (Object value, String message) - { - if (value == null) { - throw new IllegalStateException(message); - } - } - - protected void requireNull (Object value, String message) - { - if (value != null) { - throw new IllegalStateException(message); - } - } - protected void assertValidDelete () { - requireNotNull(_where, "Where clause must be specified for delete."); - requireNull(_joins, "Join clauses not supported by delete."); - requireNull(_orderBy, "OrderBy clause not applicable for delete."); - requireNull(_groupBy, "GroupBy clause not applicable for delete."); - requireNull(_limit, "Limit clause not supported by delete."); - requireNull(_fromOverride, "FromOverride clause not applicable for delete."); - requireNull(_fieldDefs, "FieldDefinition clauses not applicable for delete."); - requireNull(_forUpdate, "ForUpdate clause not supported by delete."); + checkState(_where != null, "Where clause must be specified for delete."); + checkState(_joins == null, "Join clauses not supported by delete."); + checkState(_orderBy == null, "OrderBy clause not applicable for delete."); + checkState(_groupBy == null, "GroupBy clause not applicable for delete."); + checkState(_limit == null, "Limit clause not supported by delete."); + checkState(_fromOverride == null, "FromOverride clause not applicable for delete."); + checkState(_fieldDefs == null, "FieldDefinition clauses not applicable for delete."); + checkState(_forUpdate == null, "ForUpdate clause not supported by delete."); } protected final PersistenceContext _ctx; diff --git a/src/main/java/com/samskivert/depot/SchemaMigration.java b/src/main/java/com/samskivert/depot/SchemaMigration.java index 155ddc9..6b0d415 100644 --- a/src/main/java/com/samskivert/depot/SchemaMigration.java +++ b/src/main/java/com/samskivert/depot/SchemaMigration.java @@ -32,6 +32,7 @@ import com.samskivert.depot.expression.ColumnExp; import com.samskivert.depot.impl.FieldMarshaller; import com.samskivert.depot.impl.Modifier; +import static com.google.common.base.Preconditions.checkArgument; import static com.samskivert.depot.Log.log; /** @@ -97,25 +98,20 @@ public abstract class SchemaMigration extends Modifier @Override protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException { if (!liaison.tableContainsColumn(conn, _tableName, _oldColumnName)) { - if (liaison.tableContainsColumn(conn, _tableName, _newColumnName)) { - // we'll accept this inconsistency - log.warning(_tableName + "." + _oldColumnName + " already renamed to " + - _newColumnName + "."); - return 0; - } - // but this is not OK - throw new IllegalArgumentException( - _tableName + " does not contain '" + _oldColumnName + "'"); + // if the new column already exists, log a warning, otherwise freak out + checkArgument(liaison.tableContainsColumn(conn, _tableName, _newColumnName), + _tableName + " does not contain '" + _oldColumnName + "'"); + log.warning(_tableName + "." + _oldColumnName + " already renamed to " + + _newColumnName + "."); + return 0; } // nor is this - if (liaison.tableContainsColumn(conn, _tableName, _newColumnName)) { - throw new IllegalArgumentException( - _tableName + " already contains '" + _newColumnName + "'"); - } + checkArgument(!liaison.tableContainsColumn(conn, _tableName, _newColumnName), + _tableName + " already contains '" + _newColumnName + "'"); log.info("Renaming '" + _oldColumnName + "' to '" + _newColumnName + "' in: " + - _tableName); + _tableName); return liaison.renameColumn( conn, _tableName, _oldColumnName, _newColumnName, _newColumnDef) ? 1 : 0; } @@ -274,10 +270,8 @@ public abstract class SchemaMigration extends Modifier Map> marshallers, String fieldName) { FieldMarshaller marsh = marshallers.get(fieldName); - if (marsh == null) { - throw new IllegalArgumentException( - "'" + _tableName + "' does not contain field '" + fieldName + "'"); - } + checkArgument(marsh != null, + "'" + _tableName + "' does not contain field '" + fieldName + "'"); return marsh; } diff --git a/src/main/java/com/samskivert/depot/clause/SelectClause.java b/src/main/java/com/samskivert/depot/clause/SelectClause.java index 1509707..c2c730d 100644 --- a/src/main/java/com/samskivert/depot/clause/SelectClause.java +++ b/src/main/java/com/samskivert/depot/clause/SelectClause.java @@ -32,6 +32,8 @@ import com.samskivert.depot.PersistentRecord; import com.samskivert.depot.expression.ColumnExp; import com.samskivert.depot.impl.FragmentVisitor; +import static com.google.common.base.Preconditions.checkArgument; + /** * Represents a complete select clause. */ @@ -64,17 +66,12 @@ public class SelectClause continue; } if (clause instanceof WhereClause) { - if (_where != null) { - throw new IllegalArgumentException( - "Query can't contain multiple Where clauses."); - } + checkArgument(_where == null, "Query can't contain multiple Where clauses."); _where = (WhereClause) clause; } else if (clause instanceof FromOverride) { - if (_fromOverride != null) { - throw new IllegalArgumentException( - "Query can't contain multiple FromOverride clauses."); - } + checkArgument(_fromOverride == null, + "Query can't contain multiple FromOverride clauses."); _fromOverride = (FromOverride) clause; } else if (clause instanceof Join) { @@ -84,31 +81,20 @@ public class SelectClause _disMap.put(((FieldDefinition) clause).getField(), ((FieldDefinition) clause)); } else if (clause instanceof OrderBy) { - if (_orderBy != null) { - throw new IllegalArgumentException( - "Query can't contain multiple OrderBy clauses."); - } + checkArgument(_orderBy == null, "Query can't contain multiple OrderBy clauses."); _orderBy = (OrderBy) clause; } else if (clause instanceof GroupBy) { - if (_groupBy != null) { - throw new IllegalArgumentException( - "Query can't contain multiple GroupBy clauses."); - } + checkArgument(_groupBy == null, "Query can't contain multiple GroupBy clauses."); _groupBy = (GroupBy) clause; } else if (clause instanceof Limit) { - if (_limit != null) { - throw new IllegalArgumentException( - "Query can't contain multiple Limit clauses."); - } + checkArgument(_limit == null, "Query can't contain multiple Limit clauses."); _limit = (Limit) clause; } else if (clause instanceof ForUpdate) { - if (_forUpdate != null) { - throw new IllegalArgumentException( - "Query can't contain multiple For Update clauses."); - } + checkArgument(_forUpdate == null, + "Query can't contain multiple For Update clauses."); _forUpdate = (ForUpdate) clause; } else { diff --git a/src/main/java/com/samskivert/depot/clause/WhereClause.java b/src/main/java/com/samskivert/depot/clause/WhereClause.java index a1dba08..2f16994 100644 --- a/src/main/java/com/samskivert/depot/clause/WhereClause.java +++ b/src/main/java/com/samskivert/depot/clause/WhereClause.java @@ -22,6 +22,8 @@ package com.samskivert.depot.clause; import com.samskivert.depot.expression.SQLExpression; +import static com.google.common.base.Preconditions.checkArgument; + /** * Currently only exists as a type without any functionality of its own. */ @@ -50,10 +52,8 @@ public abstract class WhereClause implements QueryClause */ protected void validateTypesMatch (Class qClass, Class kClass) { - if (!qClass.equals(kClass)) { - throw new IllegalArgumentException( - "Class mismatch between persistent record and key in query " + - "[qtype=" + qClass.getSimpleName() + ", ktype=" + kClass.getSimpleName() + "]."); - } + checkArgument(qClass.equals(kClass), + "Class mismatch between persistent record and key in query " + + "[qtype=%s, ktype=%s].", qClass.getSimpleName(), kClass.getSimpleName()); } }