Hi,
we should consider applying the attached patch to Torque before
releasing 3.1.1 I ran Findbugs on an application of mine with roughly
80 peer and object classes and this patch reduces the number of
warnings / error reports from about 500 to one (which is bogus. :-)
This is mainly working around things like new String() (replacing it
with ""), new Boolean(true) (replacing it with Boolean.TRUE). There
are some a == b checks where a and b are Strings in the peers and
doing some cleanup on constructs like this:
if (foo) {
#if ($templateFoo)
do something
#end
}
which results in
if (foo) {
}
when $templateFoo is false.
I'm sure that this does not break anything and it makes findbugs
usable on applications using Torque. Scott, your call. I would apply
it. :-)
Regards
Henning
Index: src/generator/src/java/org/apache/torque/engine/database/model/TypeMap.java
===================================================================
--- src/generator/src/java/org/apache/torque/engine/database/model/TypeMap.java
(revision 2582)
+++ src/generator/src/java/org/apache/torque/engine/database/model/TypeMap.java
(working copy)
@@ -98,13 +98,13 @@
CHAR, VARCHAR, LONGVARCHAR, CLOB, DATE, TIME, TIMESTAMP, BOOLEANCHAR
};
- public static final String CHAR_OBJECT_TYPE = "new String()";
- public static final String VARCHAR_OBJECT_TYPE = "new String()";
- public static final String LONGVARCHAR_OBJECT_TYPE = "new String()";
- public static final String CLOB_OBJECT_TYPE = "new String()";
+ public static final String CHAR_OBJECT_TYPE = "\"\"";
+ public static final String VARCHAR_OBJECT_TYPE = "\"\"";
+ public static final String LONGVARCHAR_OBJECT_TYPE = "\"\"";
+ public static final String CLOB_OBJECT_TYPE = "\"\"";
public static final String NUMERIC_OBJECT_TYPE = "new BigDecimal(0)";
public static final String DECIMAL_OBJECT_TYPE = "new BigDecimal(0)";
- public static final String BIT_OBJECT_TYPE = "new Boolean(true)";
+ public static final String BIT_OBJECT_TYPE = "Boolean.TRUE";
public static final String TINYINT_OBJECT_TYPE = "new Byte((byte)0)";
public static final String SMALLINT_OBJECT_TYPE = "new Short((short)0)";
public static final String INTEGER_OBJECT_TYPE = "new Integer(0)";
@@ -119,7 +119,7 @@
public static final String DATE_OBJECT_TYPE = "new Date()";
public static final String TIME_OBJECT_TYPE = "new Date()";
public static final String TIMESTAMP_OBJECT_TYPE = "new Date()";
- public static final String BOOLEANCHAR_OBJECT_TYPE = "new String()";
+ public static final String BOOLEANCHAR_OBJECT_TYPE = "\"\"";
public static final String BOOLEANINT_OBJECT_TYPE = "new Integer(0)";
public static final String CHAR_NATIVE_TYPE = "String";
Index: src/generator/src/templates/om/Object.vm
===================================================================
--- src/generator/src/templates/om/Object.vm (revision 2582)
+++ src/generator/src/templates/om/Object.vm (working copy)
@@ -917,7 +917,7 @@
#elseif ($cjtype == "double")
return new Double(${col.GetterName} ());
#elseif ($cjtype == "boolean")
- return new Boolean(${col.GetterName} ());
+ return Boolean.valueOf(${col.GetterName} ());
#elseif ($cjtype == "short")
return new Short(${col.GetterName} ());
#elseif ($cjtype == "byte")
@@ -957,7 +957,7 @@
#elseif ($cjtype == "double")
return new Double(${col.GetterName} ());
#elseif ($cjtype == "boolean")
- return new Boolean(${col.GetterName} ());
+ return Boolean.valueOf(${col.GetterName} ());
#elseif ($cjtype == "short")
return new Short(${col.GetterName} ());
#elseif ($cjtype == "byte")
@@ -996,7 +996,7 @@
#elseif ($cjtype == "double")
return new Double(${col.GetterName} ());
#elseif ($cjtype == "boolean")
- return new Boolean(${col.GetterName} ());
+ return Boolean.valueOf(${col.GetterName} ());
#elseif ($cjtype == "short")
return new Short(${col.GetterName} ());
#elseif ($cjtype == "byte")
Index: src/generator/src/templates/om/Peer.vm
===================================================================
--- src/generator/src/templates/om/Peer.vm (revision 2582)
+++ src/generator/src/templates/om/Peer.vm (working copy)
@@ -255,14 +255,7 @@
Object possibleBoolean = criteria.get($cup);
if (possibleBoolean instanceof Boolean)
{
- if (((Boolean) possibleBoolean).booleanValue())
- {
- criteria.add($cup, 1);
- }
- else
- {
- criteria.add($cup, 0);
- }
+ criteria.add($cup, (((Boolean) possibleBoolean).booleanValue() ? 1 :
0);
}
}
#elseif ($col.isBooleanChar())
@@ -272,26 +265,14 @@
Object possibleBoolean = criteria.get($cup);
if (possibleBoolean instanceof Boolean)
{
- if (((Boolean) possibleBoolean).booleanValue())
- {
- criteria.add($cup, "Y");
- }
- else
- {
- criteria.add($cup, "N");
- }
+ criteria.add($cup, (((Boolean) possibleBoolean).booleanValue() ? "Y" :
"N");
}
}
#end
#end
- // Set the correct dbName if it has not been overridden
- // criteria.getDbName will return the same object if not set to
- // another value so == check is okay and faster
- if (criteria.getDbName() == Torque.getDefaultDB())
- {
- criteria.setDbName(DATABASE_NAME);
- }
+ setDbName(criteria);
+
if (con == null)
{
return BasePeer.doInsert(criteria);
@@ -451,14 +432,7 @@
Object possibleBoolean = criteria.get($cup);
if (possibleBoolean instanceof Boolean)
{
- if (((Boolean) possibleBoolean).booleanValue())
- {
- criteria.add($cup, 1);
- }
- else
- {
- criteria.add($cup, 0);
- }
+ criteria.add($cup, (((Boolean) possibleBoolean).booleanValue() ? 1 :
0);
}
}
#elseif ($col.isBooleanChar())
@@ -468,26 +442,14 @@
Object possibleBoolean = criteria.get($cup);
if (possibleBoolean instanceof Boolean)
{
- if (((Boolean) possibleBoolean).booleanValue())
- {
- criteria.add($cup, "Y");
- }
- else
- {
- criteria.add($cup, "N");
- }
+ criteria.add($cup, (((Boolean) possibleBoolean).booleanValue() ? "Y" :
"N");
}
}
#end
#end
- // Set the correct dbName if it has not been overridden
- // criteria.getDbName will return the same object if not set to
- // another value so == check is okay and faster
- if (criteria.getDbName() == Torque.getDefaultDB())
- {
- criteria.setDbName(DATABASE_NAME);
- }
+ setDbName(criteria);
+
// BasePeer returns a List of Value (Village) arrays. The array
// order follows the order columns were placed in the Select clause.
if (con == null)
@@ -639,14 +601,7 @@
Object possibleBoolean = criteria.get($cup);
if (possibleBoolean instanceof Boolean)
{
- if (((Boolean) possibleBoolean).booleanValue())
- {
- criteria.add($cup, 1);
- }
- else
- {
- criteria.add($cup, 0);
- }
+ criteria.add($cup, (((Boolean) possibleBoolean).booleanValue() ? 1 :
0);
}
}
#elseif ($col.isBooleanChar())
@@ -656,14 +611,7 @@
Object possibleBoolean = criteria.get($cup);
if (possibleBoolean instanceof Boolean)
{
- if (((Boolean) possibleBoolean).booleanValue())
- {
- criteria.add($cup, "Y");
- }
- else
- {
- criteria.add($cup, "N");
- }
+ criteria.add($cup, (((Boolean) possibleBoolean).booleanValue() ? "Y" :
"N");
}
}
#end
@@ -672,13 +620,8 @@
#end
#end
- // Set the correct dbName if it has not been overridden
- // criteria.getDbName will return the same object if not set to
- // another value so == check is okay and faster
- if (criteria.getDbName() == Torque.getDefaultDB())
- {
- criteria.setDbName(DATABASE_NAME);
- }
+ setDbName(criteria);
+
if (con == null)
{
BasePeer.doUpdate(selectCriteria, criteria);
@@ -724,14 +667,7 @@
Object possibleBoolean = criteria.get($cup);
if (possibleBoolean instanceof Boolean)
{
- if (((Boolean) possibleBoolean).booleanValue())
- {
- criteria.add($cup, 1);
- }
- else
- {
- criteria.add($cup, 0);
- }
+ criteria.add($cup, (((Boolean) possibleBoolean).booleanValue() ? 1 :
0);
}
}
#elseif ($col.isBooleanChar())
@@ -741,26 +677,14 @@
Object possibleBoolean = criteria.get($cup);
if (possibleBoolean instanceof Boolean)
{
- if (((Boolean) possibleBoolean).booleanValue())
- {
- criteria.add($cup, "Y");
- }
- else
- {
- criteria.add($cup, "N");
- }
+ criteria.add($cup, (((Boolean) possibleBoolean).booleanValue() ? "Y" :
"N");
}
}
#end
#end
- // Set the correct dbName if it has not been overridden
- // criteria.getDbName will return the same object if not set to
- // another value so == check is okay and faster
- if (criteria.getDbName() == Torque.getDefaultDB())
- {
- criteria.setDbName(DATABASE_NAME);
- }
+ setDbName(criteria);
+
if (con == null)
{
BasePeer.doDelete(criteria);
@@ -1229,27 +1153,21 @@
* @throws TorqueException Any exceptions caught during processing will be
* rethrown wrapped into a TorqueException.
*/
- protected static List doSelectJoin${joinColumnId}(Criteria c)
+ protected static List doSelectJoin${joinColumnId}(Criteria criteria)
throws TorqueException
{
- // Set the correct dbName if it has not been overridden
- // c.getDbName will return the same object if not set to
- // another value so == check is okay and faster
- if (c.getDbName() == Torque.getDefaultDB())
- {
- c.setDbName(DATABASE_NAME);
- }
+ setDbName(criteria);
- ${table.JavaName}Peer.addSelectColumns(c);
+ ${table.JavaName}Peer.addSelectColumns(criteria);
int offset = numColumns + 1;
- ${joinClassName}Peer.addSelectColumns(c);
+ ${joinClassName}Peer.addSelectColumns(criteria);
#set ( $lfMap = $fk.LocalForeignMapping )
#foreach ($columnName in $fk.LocalColumns)
#set ( $column = $table.getColumn($columnName) )
#set ( $columnFk = $joinTable.getColumn( $lfMap.get($columnName) ) )
- c.addJoin(${table.JavaName}Peer.$column.Name.toUpperCase(),
+ criteria.addJoin(${table.JavaName}Peer.$column.Name.toUpperCase(),
${joinClassName}Peer.$columnFk.Name.toUpperCase());
#end
@@ -1258,42 +1176,28 @@
#set ( $cup=$col.Name.toUpperCase() )
#if($col.isBooleanInt())
// check for conversion from boolean to int
- if (c.containsKey($cup))
+ if (criteria.containsKey($cup))
{
- Object possibleBoolean = c.get($cup);
+ Object possibleBoolean = criteria.get($cup);
if (possibleBoolean instanceof Boolean)
{
- if (((Boolean) possibleBoolean).booleanValue())
- {
- c.add($cup, 1);
- }
- else
- {
- c.add($cup, 0);
- }
+ criteria.add($cup, (((Boolean) possibleBoolean).booleanValue() ? 1 :
0);
}
}
#elseif ($col.isBooleanChar())
// check for conversion from boolean to Y/N
- if ( c.containsKey($cup) )
+ if ( criteria.containsKey($cup) )
{
- Object possibleBoolean = c.get($cup);
+ Object possibleBoolean = criteria.get($cup);
if (possibleBoolean instanceof Boolean)
{
- if (((Boolean) possibleBoolean).booleanValue())
- {
- c.add($cup, "Y");
- }
- else
- {
- c.add($cup, "N");
- }
+ criteria.add($cup, (((Boolean) possibleBoolean).booleanValue() ? "Y" :
"N");
}
}
#end
#end
- List rows = BasePeer.doSelect(c);
+ List rows = BasePeer.doSelect(criteria);
List results = new ArrayList();
for (int i = 0; i < rows.size(); i++)
@@ -1332,13 +1236,14 @@
break;
}
}
+
+ #if ($objectIsCaching)
if (newObject)
{
- #if ($objectIsCaching)
obj2.init${collThisTable}();
obj2.add${collThisTableMs}(obj1);
- #end
}
+ #end
results.add(obj1);
}
return results;
@@ -1396,18 +1301,12 @@
* @throws TorqueException Any exceptions caught during processing will be
* rethrown wrapped into a TorqueException.
*/
- protected static List doSelectJoinAllExcept${excludeString}(Criteria c)
+ protected static List doSelectJoinAllExcept${excludeString}(Criteria criteria)
throws TorqueException
{
- // Set the correct dbName if it has not been overridden
- // c.getDbName will return the same object if not set to another value
- // so == check is okay and faster
- if (c.getDbName() == Torque.getDefaultDB())
- {
- c.setDbName(DATABASE_NAME);
- }
+ setDbName(criteria);
- addSelectColumns(c);
+ addSelectColumns(criteria);
int offset2 = numColumns + 1;
#set ( $index = 2 )
#foreach ($fk in $table.ForeignKeys)
@@ -1418,7 +1317,7 @@
#if (!$joinClassName.equals($excludeClassName))
#set ( $new_index = $index + 1 )
- ${joinClassName}Peer.addSelectColumns(c);
+ ${joinClassName}Peer.addSelectColumns(criteria);
int offset$new_index = offset$index + ${joinClassName}Peer.numColumns;
#set ( $index = $new_index )
#end
@@ -1428,42 +1327,28 @@
#set ( $cup=$col.Name.toUpperCase() )
#if($col.isBooleanInt())
// check for conversion from boolean to int
- if (c.containsKey($cup))
+ if (criteria.containsKey($cup))
{
- Object possibleBoolean = c.get($cup);
+ Object possibleBoolean = criteria.get($cup);
if (possibleBoolean instanceof Boolean)
{
- if (((Boolean) possibleBoolean).booleanValue())
- {
- c.add($cup, 1);
- }
- else
- {
- c.add($cup, 0);
- }
+ criteria.add($cup, (((Boolean) possibleBoolean).booleanValue() ? 1 :
0);
}
}
#elseif ($col.isBooleanChar())
// check for conversion from boolean to Y/N
- if ( c.containsKey($cup) )
+ if ( criteria.containsKey($cup) )
{
- Object possibleBoolean = c.get($cup);
+ Object possibleBoolean = criteria.get($cup);
if (possibleBoolean instanceof Boolean)
{
- if (((Boolean) possibleBoolean).booleanValue())
- {
- c.add($cup, "Y");
- }
- else
- {
- c.add($cup, "N");
- }
+ criteria.add($cup, (((Boolean) possibleBoolean).booleanValue() ? "Y" :
"N");
}
}
#end
#end
- List rows = BasePeer.doSelect(c);
+ List rows = BasePeer.doSelect(criteria);
List results = new ArrayList();
for (int i = 0; i < rows.size(); i++)
@@ -1534,13 +1419,13 @@
break;
}
}
+ #if ($objectIsCaching)
if (newObject)
{
- #if ($objectIsCaching)
obj${index}.init${collThisTable}();
obj${index}.add${collThisTableMs}(obj1);
- #end
}
+ #end
#end
#end
#end
@@ -1569,4 +1454,15 @@
return Torque.getDatabaseMap(DATABASE_NAME).getTable(TABLE_NAME);
}
#end ## ends if (!$table.isAlias())
+
+ private static void setDbName(Criteria crit)
+ {
+ if (crit != null && crit.getDbName() != null)
+ {
+ if (crit.getDbName().equals(Torque.getDefaultDB()))
+ {
+ crit.setDbName(DATABASE_NAME);
+ }
+ }
+ }
}
