From ce71fa01fa80936df378566ff4cc17a3cee7cdb1 Mon Sep 17 00:00:00 2001 From: Michael Bayne Date: Wed, 8 Dec 2010 20:24:36 +0000 Subject: [PATCH] More use of Preconditions. --- .../com/samskivert/depot/DepotRepository.java | 14 ++--- .../samskivert/depot/impl/BuildVisitor.java | 36 +++++------- .../depot/impl/DepotMarshaller.java | 55 +++++++------------ .../com/samskivert/depot/impl/DepotTypes.java | 15 ++--- .../depot/impl/FieldMarshaller.java | 28 ++++------ .../samskivert/depot/impl/FindAllQuery.java | 13 ++--- .../depot/impl/PostgreSQLBuilder.java | 3 +- .../com/samskivert/depot/impl/SQLBuilder.java | 12 ++-- .../samskivert/depot/util/RuntimeUtil.java | 17 +++--- 9 files changed, 70 insertions(+), 123 deletions(-) diff --git a/src/main/java/com/samskivert/depot/DepotRepository.java b/src/main/java/com/samskivert/depot/DepotRepository.java index 6954ba4..6028d4a 100644 --- a/src/main/java/com/samskivert/depot/DepotRepository.java +++ b/src/main/java/com/samskivert/depot/DepotRepository.java @@ -62,6 +62,7 @@ import com.samskivert.depot.impl.clause.UpdateClause; import com.samskivert.depot.impl.expression.ValueExp; import com.samskivert.depot.impl.util.SeqImpl; +import static com.google.common.base.Preconditions.checkArgument; import static com.samskivert.depot.Log.log; /** @@ -275,7 +276,6 @@ public abstract class DepotRepository } if (cache == CacheStrategy.BEST) { cache = (reason != null) ? CacheStrategy.NONE : CacheStrategy.SHORT_KEYS; - } else if (reason != null) { // if user explicitly asked for a strategy we can't do, protest throw new IllegalArgumentException( @@ -413,9 +413,7 @@ public abstract class DepotRepository requireNotComputed(pClass, "update"); DepotMarshaller marsh = _ctx.getMarshaller(pClass); Key key = marsh.getPrimaryKey(record); - if (key == null) { - throw new IllegalArgumentException("Can't update record with null primary key."); - } + checkArgument(key != null, "Can't update record with null primary key."); return doUpdate(key, new UpdateClause(pClass, key, marsh.getColumnFieldNames(), record)); } @@ -436,9 +434,7 @@ public abstract class DepotRepository requireNotComputed(pClass, "update"); DepotMarshaller marsh = _ctx.getMarshaller(pClass); Key key = marsh.getPrimaryKey(record); - if (key == null) { - throw new IllegalArgumentException("Can't update record with null primary key."); - } + checkArgument(key != null, "Can't update record with null primary key."); return doUpdate(key, new UpdateClause(pClass, key, modifiedFields, record)); } @@ -667,9 +663,7 @@ public abstract class DepotRepository { @SuppressWarnings("unchecked") Class type = (Class)record.getClass(); Key primaryKey = _ctx.getMarshaller(type).getPrimaryKey(record); - if (primaryKey == null) { - throw new IllegalArgumentException("Can't delete record with null primary key."); - } + checkArgument(primaryKey != null, "Can't delete record with null primary key."); return delete(primaryKey); } diff --git a/src/main/java/com/samskivert/depot/impl/BuildVisitor.java b/src/main/java/com/samskivert/depot/impl/BuildVisitor.java index b24fcc4..4e6d510 100644 --- a/src/main/java/com/samskivert/depot/impl/BuildVisitor.java +++ b/src/main/java/com/samskivert/depot/impl/BuildVisitor.java @@ -98,6 +98,7 @@ import com.samskivert.depot.impl.operator.IsNull; import com.samskivert.depot.impl.operator.MultiOperator; import com.samskivert.depot.impl.operator.Not; +import static com.google.common.base.Preconditions.checkArgument; import static com.samskivert.Log.log; /** @@ -355,10 +356,8 @@ public abstract class BuildVisitor implements FragmentVisitor } _builder.append("select "); - if (_definitions.containsKey(pClass)) { - throw new IllegalArgumentException( - "Can not yet nest SELECTs on the same persistent record."); - } + checkArgument(!_definitions.containsKey(pClass), + "Can not yet nest SELECTs on the same persistent record."); Map definitionMap = Maps.newHashMap(); for (FieldDefinition definition : selectClause.getFieldDefinitions()) { @@ -444,10 +443,8 @@ public abstract class BuildVisitor implements FragmentVisitor public Void visit (UpdateClause updateClause) { - if (updateClause.getWhereClause() == null) { - throw new IllegalArgumentException( - "I dare not currently perform UPDATE without a WHERE clause."); - } + checkArgument(updateClause.getWhereClause() != null, + "I dare not currently perform UPDATE without a WHERE clause."); Class pClass = updateClause.getPersistentClass(); _innerClause = true; @@ -785,10 +782,8 @@ public abstract class BuildVisitor implements FragmentVisitor { // TODO: nix type and use the class from the supplied ColumnExp DepotMarshaller dm = _types.getMarshaller(type); - if (dm == null) { - throw new IllegalArgumentException( - "Unknown field on persistent record [record=" + type + ", field=" + field + "]"); - } + checkArgument(dm != null, "Unknown field on persistent record [record=%s, field=%s]", + type, field); FieldMarshaller fm = dm.getFieldMarshaller(field.name); appendIdentifier(fm.getColumnName()); @@ -800,10 +795,8 @@ public abstract class BuildVisitor implements FragmentVisitor { Class type = field.getPersistentClass(); DepotMarshaller dm = _types.getMarshaller(type); - if (dm == null) { - throw new IllegalArgumentException( - "Unknown field on persistent record [record=" + type + ", field=" + field + "]"); - } + checkArgument(dm != null, "Unknown field on persistent record [record=%s, field=%s]", + type, field); // first, see if there's a field definition FieldMarshaller fm = dm.getFieldMarshaller(field.name); @@ -813,15 +806,12 @@ public abstract class BuildVisitor implements FragmentVisitor if (fieldDef != null) { boolean useOverride; if (fieldDef instanceof FieldOverride) { - if (fm.getComputed() != null && dm.getComputed() != null) { - throw new IllegalArgumentException( - "FieldOverride cannot be used on @Computed field: " + field); - } + checkArgument(fm.getComputed() == null || dm.getComputed() == null, + "FieldOverride cannot be used on @Computed field: " + field); useOverride = _enableOverrides; - } else if (fm.getComputed() == null && dm.getComputed() == null) { - throw new IllegalArgumentException( - "FieldDefinition must not be used on concrete field: " + field); } else { + checkArgument(fm.getComputed() != null || dm.getComputed() != null, + "FieldDefinition must not be used on concrete field: " + field); useOverride = true; } diff --git a/src/main/java/com/samskivert/depot/impl/DepotMarshaller.java b/src/main/java/com/samskivert/depot/impl/DepotMarshaller.java index d6f35a4..049c33f 100644 --- a/src/main/java/com/samskivert/depot/impl/DepotMarshaller.java +++ b/src/main/java/com/samskivert/depot/impl/DepotMarshaller.java @@ -70,6 +70,7 @@ import com.samskivert.depot.expression.ColumnExp; import com.samskivert.depot.expression.SQLExpression; import com.samskivert.depot.impl.clause.CreateIndexClause; +import static com.google.common.base.Preconditions.checkArgument; import static com.samskivert.depot.Log.log; /** @@ -90,8 +91,8 @@ public class DepotMarshaller implements QueryMarshal { _pClass = pClass; - Preconditions.checkArgument(!java.lang.reflect.Modifier.isAbstract(pClass.getModifiers()), - "Can't handle reference to abstract record: " + pClass.getName()); + checkArgument(!java.lang.reflect.Modifier.isAbstract(pClass.getModifiers()), + "Can't handle reference to abstract record: " + pClass.getName()); Entity entity = pClass.getAnnotation(Entity.class); @@ -173,17 +174,13 @@ public class DepotMarshaller implements QueryMarshal ftype.equals(Short.TYPE) || ftype.equals(Short.class) || ftype.equals(Integer.TYPE) || ftype.equals(Integer.class) || ftype.equals(Long.TYPE) || ftype.equals(Long.class)); - if (!isNumeric) { - throw new IllegalArgumentException( - "Cannot use @GeneratedValue on non-numeric column: " + field.getName()); - } + checkArgument(isNumeric, "Cannot use @GeneratedValue on non-numeric column: %s", + field.getName()); switch(gv.strategy()) { case AUTO: case IDENTITY: - if (seenIdentityGenerator) { - throw new IllegalArgumentException( - "Persistent records can have at most one AUTO/IDENTITY generator."); - } + checkArgument(!seenIdentityGenerator, "Persistent records can have at " + + "most one AUTO/IDENTITY generator."); _valueGenerators.put(field.getName(), new IdentityValueGenerator(gv, this, fm)); seenIdentityGenerator = true; break; @@ -191,10 +188,7 @@ public class DepotMarshaller implements QueryMarshal case TABLE: String name = gv.generator(); generator = context.tableGenerators.get(name); - if (generator == null) { - throw new IllegalArgumentException( - "Unknown generator [generator=" + name + "]"); - } + checkArgument(generator != null, "Unknown generator [generator=" + name + "]"); _valueGenerators.put( field.getName(), new TableValueGenerator(generator, gv, this, fm)); break; @@ -212,13 +206,12 @@ public class DepotMarshaller implements QueryMarshal Tuple entry = new Tuple(fieldColumn, Order.ASC); if (index.unique()) { - Preconditions.checkArgument(!namedFieldIndices.containsKey(index.name()), - "All @Index for a particular name must be unique or non-unique"); + checkArgument(!namedFieldIndices.containsKey(index.name()), + "All @Index for a particular name must be unique or non-unique"); uniqueNamedFieldIndices.put(name, entry); } else { - Preconditions.checkArgument( - !uniqueNamedFieldIndices.containsKey(index.name()), - "All @Index for a particular name must be unique or non-unique"); + checkArgument(!uniqueNamedFieldIndices.containsKey(index.name()), + "All @Index for a particular name must be unique or non-unique"); namedFieldIndices.put(name, entry); } } @@ -258,10 +251,7 @@ public class DepotMarshaller implements QueryMarshal int ii = 0; for (String field : constraint.fields()) { FieldMarshaller fm = _fields.get(field); - if (fm == null) { - throw new IllegalArgumentException( - "Unknown unique constraint field: " + field); - } + checkArgument(fm != null, "Unknown unique constraint field: " + field); colExps[ii ++] = new ColumnExp(_pClass, field); } _indexes.add(buildIndex(constraint.name(), true, colExps)); @@ -338,10 +328,8 @@ public class DepotMarshaller implements QueryMarshal public FullTextIndex getFullTextIndex (String name) { FullTextIndex fti = _fullTextIndexes.get(name); - if (fti == null) { - throw new IllegalStateException("Persistent class missing full text index " + - "[class=" + _pClass + ", index=" + name + "]"); - } + checkArgument(fti != null, "Persistent class missing full text index " + + "[class=" + _pClass + ", index=" + name + "]"); return fti; } @@ -486,10 +474,9 @@ public class DepotMarshaller implements QueryMarshal Comparable[] values = new Comparable[_pkColumns.size()]; for (int ii = 0; ii < _pkColumns.size(); ii++) { Object keyValue = _pkColumns.get(ii).getFromSet(rs); - if (!(keyValue instanceof Comparable)) { - throw new IllegalArgumentException("Key field must be Comparable [field=" + - _pkColumns.get(ii).getColumnName() + "]"); - } + checkArgument((keyValue instanceof Comparable), + "Key field must be Comparable [field=%s]", + _pkColumns.get(ii).getColumnName()); values[ii] = (Comparable) keyValue; } return new Key(_pClass, values); @@ -999,10 +986,8 @@ public class DepotMarshaller implements QueryMarshal String[] columns = new String[fields.length]; for (int ii = 0; ii < columns.length; ii ++) { FieldMarshaller fm = _fields.get(fields[ii].name); - if (fm == null) { - throw new IllegalArgumentException( - "Unknown field on record [field=" + fields[ii] + ", class=" + _pClass + "]"); - } + checkArgument(fm != null, "Unknown field on record [field=%s, class=%s]", + fields[ii], _pClass); columns[ii] = fm.getColumnName(); } return columns; diff --git a/src/main/java/com/samskivert/depot/impl/DepotTypes.java b/src/main/java/com/samskivert/depot/impl/DepotTypes.java index 7e48090..2034607 100644 --- a/src/main/java/com/samskivert/depot/impl/DepotTypes.java +++ b/src/main/java/com/samskivert/depot/impl/DepotTypes.java @@ -35,6 +35,8 @@ import com.samskivert.depot.PersistentRecord; import com.samskivert.depot.clause.QueryClause; import com.samskivert.depot.expression.SQLExpression; +import static com.google.common.base.Preconditions.checkArgument; + /** * Maintains a record of the persistent classes brought into the context of the associated SQL, * i.e. any class associated with a concrete table that would appear in FROM or JOIN clauses or as @@ -121,9 +123,7 @@ public class DepotTypes { if (_useTableAbbreviations) { Integer ix = _classIx.get(cl); - if (ix == null) { - throw new IllegalArgumentException("Unknown persistence class: " + cl); - } + checkArgument(ix != null, "Unknown persistence class: " + cl); return "T" + (ix+1); } return getTableName(cl); @@ -140,10 +140,7 @@ public class DepotTypes public String getColumnName (Class cl, String field) { FieldMarshaller fm = getMarshaller(cl).getFieldMarshaller(field); - if (fm == null) { - throw new IllegalArgumentException( - "Field not known on class [field=" + field + ", class=" + cl + "]"); - } + checkArgument(fm != null, "Field not known on class [field=%s, class=%s]", field, cl); return fm.getColumnName(); } @@ -156,9 +153,7 @@ public class DepotTypes public DepotMarshaller getMarshaller (Class cl) { DepotMarshaller marsh = _classMap.get(cl); - if (marsh == null) { - throw new IllegalArgumentException("Persistent class not known: " + cl); - } + checkArgument(marsh != null, "Persistent class not known: " + cl); return marsh; } diff --git a/src/main/java/com/samskivert/depot/impl/FieldMarshaller.java b/src/main/java/com/samskivert/depot/impl/FieldMarshaller.java index 79a160b..5dd42d7 100644 --- a/src/main/java/com/samskivert/depot/impl/FieldMarshaller.java +++ b/src/main/java/com/samskivert/depot/impl/FieldMarshaller.java @@ -47,6 +47,8 @@ import com.samskivert.util.ByteEnumUtil; import com.samskivert.util.Logger; import com.samskivert.util.StringUtil; +import static com.google.common.base.Preconditions.checkArgument; + /** * Handles the marshalling and unmarshalling of a particular field of a persistent object. * @@ -91,9 +93,7 @@ public abstract class FieldMarshaller // finally look for an @Transform annotation on the field type (expensive) xform = findTransformAnnotation(field.getType()); - if (xform == null) { - throw new IllegalArgumentException("Cannot marshall " + field + "."); - } + checkArgument(xform != null, "Cannot marshall " + field + "."); } try { @@ -116,12 +116,9 @@ public abstract class FieldMarshaller final Transformer xformer, Field field, Transform annotation) { Class pojoType = getTransformerType(xformer, "from"); - if (!pojoType.isAssignableFrom(field.getType())) { - throw new IllegalArgumentException( - "@Transform error on " + field.getType().getName() + "." + - field.getName() + ": " + xformer.getClass().getName() + " cannot convert " + - field.getType().getName()); - } + checkArgument(pojoType.isAssignableFrom(field.getType()), + "@Transform error on %s.%s: %s cannot convert %s", field.getType().getName(), + field.getName(), xformer.getClass().getName(), field.getType().getName()); xformer.init(field.getGenericType(), annotation); @SuppressWarnings("unchecked") final FieldMarshaller delegate = @@ -455,11 +452,8 @@ public abstract class FieldMarshaller } } } - if (ttype == null) { - throw new IllegalArgumentException( - Logger.format("Transformer lacks " + which + "Persistent() method!?", - "xclass", xformer.getClass())); - } + checkArgument(ttype != null, "Transformer lacks %sPersistent() method!? [xclass=%s]", + which, xformer.getClass()); return ttype; } @@ -745,10 +739,8 @@ public abstract class FieldMarshaller @Override public void create (Field field) { super.create(field); // do some sanity checking so that the unsafe business we do below is safer - if (!Enum.class.isAssignableFrom(_eclass)) { - throw new IllegalArgumentException( - "ByteEnum not implemented by real Enum: " + field); - } + checkArgument(Enum.class.isAssignableFrom(_eclass), + "ByteEnum not implemented by real Enum: " + field); } @Override public String getColumnType (ColumnTyper typer, int length) { diff --git a/src/main/java/com/samskivert/depot/impl/FindAllQuery.java b/src/main/java/com/samskivert/depot/impl/FindAllQuery.java index 0a37441..909264a 100644 --- a/src/main/java/com/samskivert/depot/impl/FindAllQuery.java +++ b/src/main/java/com/samskivert/depot/impl/FindAllQuery.java @@ -51,6 +51,7 @@ import com.samskivert.depot.expression.ColumnExp; import com.samskivert.depot.impl.operator.In; import com.samskivert.jdbc.DatabaseLiaison; +import static com.google.common.base.Preconditions.checkArgument; import static com.samskivert.depot.Log.log; /** @@ -86,15 +87,11 @@ public abstract class FindAllQuery { super(ctx, type); - if (_dmarsh.getComputed() != null) { - throw new IllegalArgumentException( - "This algorithm doesn't work on @Computed records."); - } + checkArgument(_dmarsh.getComputed() == null, + "This algorithm doesn't work on @Computed records."); for (QueryClause clause : clauses) { - if (clause instanceof FieldOverride) { - throw new IllegalArgumentException( - "This algorithm doesn't work with FieldOverrides."); - } + checkArgument(!(clause instanceof FieldOverride), + "This algorithm doesn't work with FieldOverrides."); } _select = new SelectClause(_type, _dmarsh.getPrimaryKeyFields(), clauses); diff --git a/src/main/java/com/samskivert/depot/impl/PostgreSQLBuilder.java b/src/main/java/com/samskivert/depot/impl/PostgreSQLBuilder.java index fdbbc78..73340f5 100644 --- a/src/main/java/com/samskivert/depot/impl/PostgreSQLBuilder.java +++ b/src/main/java/com/samskivert/depot/impl/PostgreSQLBuilder.java @@ -289,8 +289,7 @@ public class PostgreSQLBuilder case English: return "pg_catalog.english"; default: - throw new IllegalArgumentException( - "Unknown full text configuration: " + configuration); + throw new IllegalArgumentException("Unknown full text configuration: " + configuration); } } diff --git a/src/main/java/com/samskivert/depot/impl/SQLBuilder.java b/src/main/java/com/samskivert/depot/impl/SQLBuilder.java index adaad73..678967f 100644 --- a/src/main/java/com/samskivert/depot/impl/SQLBuilder.java +++ b/src/main/java/com/samskivert/depot/impl/SQLBuilder.java @@ -36,6 +36,8 @@ import com.samskivert.depot.clause.QueryClause; import com.samskivert.jdbc.ColumnDefinition; import com.samskivert.util.ByteEnum; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; import static com.samskivert.depot.Log.log; /** @@ -74,9 +76,7 @@ public abstract class SQLBuilder public PreparedStatement prepare (Connection conn) throws SQLException { - if (_buildVisitor == null) { - throw new IllegalArgumentException("Cannot prepare query until it's been built."); - } + checkState(_buildVisitor != null, "Cannot prepare query until it's been built."); PreparedStatement stmt = conn.prepareStatement(_buildVisitor.getQuery()); @@ -136,10 +136,8 @@ public abstract class SQLBuilder } // sanity check nullability - if (coldef.nullable && field.getType().isPrimitive()) { - throw new IllegalArgumentException( - "Primitive Java type cannot be nullable [field=" + field.getName() + "]"); - } + checkArgument(!coldef.nullable || !field.getType().isPrimitive(), + "Primitive Java type cannot be nullable [field=" + field.getName() + "]"); return coldef; } diff --git a/src/main/java/com/samskivert/depot/util/RuntimeUtil.java b/src/main/java/com/samskivert/depot/util/RuntimeUtil.java index a465180..b15868e 100644 --- a/src/main/java/com/samskivert/depot/util/RuntimeUtil.java +++ b/src/main/java/com/samskivert/depot/util/RuntimeUtil.java @@ -35,6 +35,8 @@ import com.google.common.collect.Maps; import com.samskivert.depot.PersistentRecord; import com.samskivert.util.Tuple; +import static com.google.common.base.Preconditions.checkArgument; + /** * Creates functions that map persistent records to runtime counterparts and vice versa. A few bits * of magic are provided to help in mapping from the persistent world to the runtime work. Namely: @@ -176,12 +178,9 @@ public class RuntimeUtil } // if we have no persistent field for the runtime field, we're now out of luck - if (pfield == null) { - throw new IllegalArgumentException( - "Cannot create mapping for " + rfield + ". Neither " + - pclass.getSimpleName() + "." + rfield.getName() + " nor " + - rfield.getType() + " " + getter + "() exist."); - } + checkArgument(pfield != null, + "Cannot create mapping for %s. Neither %s.%s nor %s %s() exist.", + rfield, pclass.getSimpleName(), rfield.getName(), rfield.getType(), getter); // if the fields match exactly, return a getter that just gets the field if (rfield.getType().equals(pfield.getType())) { @@ -222,10 +221,8 @@ public class RuntimeUtil } // if we have no persistent field for this runtime field, we're now out of luck - if (pfield == null) { - throw new IllegalArgumentException("Cannot create setter for " + rfield + ". " + - "No corresponding field exists in " + pclass + "."); - } + checkArgument(pfield != null, "Cannot create setter for " + rfield + ". " + + "No corresponding field exists in " + pclass + "."); // if the fields match exactly, return a setter that just sets the field if (rfield.getType().equals(pfield.getType())) {