db-torque-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Scott Eade <se...@backstagetech.com.au>
Subject Re: Findbugs Patch for 3.1.1
Date Thu, 26 Aug 2004 02:41:09 GMT
I will apply this for 3.1.1, but it would have been easier if you had 
provided a patch against cvs rather than against your version that 
includes the boolean getter patch that I don't see going in as part of 
3.1.1 (3.1.2-dev is more acceptable I think).  I assume there will be 
some corresponding changes to ObjectWithManager.vm (I know you don't use 
it, but some do).

Scott

-- 
Scott Eade
Backstage Technologies Pty. Ltd.
http://www.backstagetech.com.au


Henning P. Schmiedehausen wrote:

>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);
>+      }
>+    }
>+  }
> }
>
>  
>

---------------------------------------------------------------------
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