db-torque-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henning P. Schmiedehausen" <...@intermeta.de>
Subject Findbugs Patch for 3.1.1
Date Wed, 25 Aug 2004 17:42:27 GMT
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);
+      }
+    }
+  }
 }

-- 
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen          INTERMETA GmbH
hps@intermeta.de        +49 9131 50 654 0   http://www.intermeta.de/

RedHat Certified Engineer -- Jakarta Turbine Development  -- hero for hire
   Linux, Java, perl, Solaris -- Consulting, Training, Development

"Fighting for one's political stand is an honorable action, but re-
 fusing to acknowledge that there might be weaknesses in one's
 position - in order to identify them so that they can be remedied -
 is a large enough problem with the Open Source movement that it
 deserves to be on this list of the top five problems."
                       -- Michelle Levesque, "Fundamental Issues with
                                    Open Source Software Development"

---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org


Mime
View raw message