Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3
    • Fix Version/s: 1.4
    • Labels:
      None
    • Environment:

      Inefficient use of keySet iterator instead of entrySet iterator
      Method invokes inefficient floating-point Number constructor; use static valueOf instead

      Description

      Index: src/test/org/apache/commons/dbutils/wrappers/SqlNullCheckedResultSetTest.java
      ===================================================================
      — src/test/org/apache/commons/dbutils/wrappers/SqlNullCheckedResultSetTest.java (revision 1025562)
      +++ src/test/org/apache/commons/dbutils/wrappers/SqlNullCheckedResultSetTest.java (working copy)
      @@ -487,7 +487,7 @@
      }
      if (getUrlInt != null && getUrlString != null) {
      assertEquals(null, getUrlInt.invoke(rs,

      • new Object[] { new Integer(1) } ) );
        + new Object[] { Integer.valueOf(1) } ) );
        assertTrue(rs.wasNull());
        assertEquals(null, getUrlString.invoke(rs,
        new Object[] { "column" } ) );
        @@ -496,7 +496,7 @@
        URL u = new URL("http://www.apache.org");
        rs2.setNullURL(u);
        assertEquals(u, getUrlInt.invoke(rs,
        - new Object[] { new Integer(1) }

        ) );
        + new Object[]

        { Integer.valueOf(1) }

        ) );
        assertEquals(u, getUrlString.invoke(rs,
        new Object[]

        { "column" }

        ) );
        }
        @@ -830,19 +830,19 @@
        return Boolean.FALSE;

      } else if (returnType.equals(Integer.TYPE))

      { - return new Integer(0); + return Integer.valueOf(0); }

      else if (returnType.equals(Short.TYPE))

      { - return new Short((short) 0); + return Short.valueOf((short) 0); }

      else if (returnType.equals(Double.TYPE))

      { return new Double(0); }

      else if (returnType.equals(Long.TYPE))

      { - return new Long(0); + return Long.valueOf(0); }

      else if (returnType.equals(Byte.TYPE))

      { - return new Byte((byte) 0); + return Byte.valueOf((byte) 0); }

      else if (returnType.equals(Float.TYPE)) {
      return new Float(0);
      Index: src/test/org/apache/commons/dbutils/BaseTestCase.java
      ===================================================================
      — src/test/org/apache/commons/dbutils/BaseTestCase.java (revision 1025562)
      +++ src/test/org/apache/commons/dbutils/BaseTestCase.java (working copy)
      @@ -56,8 +56,8 @@
      "2",
      "3",
      " notInBean ",

      • new Integer(1),
      • new Integer(2),
        + Integer.valueOf(1),
        + Integer.valueOf(2),
        null,
        null,
        new Date(),
        @@ -69,8 +69,8 @@
        "5",
        "6",
        " notInBean ",
      • new Integer(3),
      • new Integer(4),
        + Integer.valueOf(3),
        + Integer.valueOf(4),
        null,
        null,
        new Date(),
        Index: src/test/org/apache/commons/dbutils/MockResultSet.java
        ===================================================================
          • src/test/org/apache/commons/dbutils/MockResultSet.java (revision 1025562)
            +++ src/test/org/apache/commons/dbutils/MockResultSet.java (working copy)
            @@ -137,7 +137,7 @@

      try

      { return (obj == null) - ? new Byte((byte) 0) + ? Byte.valueOf((byte) 0) : Byte.valueOf(obj.toString()); }

      catch (NumberFormatException e) {
      @@ -192,7 +192,7 @@

      try

      { return (obj == null) - ? new Integer(0) + ? Integer.valueOf(0) : Integer.valueOf(obj.toString()); }

      catch (NumberFormatException e) {
      @@ -210,7 +210,7 @@
      this.setWasNull(obj);

      try

      { - return (obj == null) ? new Long(0) : Long.valueOf(obj.toString()); + return (obj == null) ? Long.valueOf(0) : Long.valueOf(obj.toString()); }

      catch (NumberFormatException e) {
      throw new SQLException(e.getMessage());
      @@ -243,7 +243,7 @@

      try

      { return (obj == null) - ? new Short((short) 0) + ? Short.valueOf((short) 0) : Short.valueOf(obj.toString()); }

      catch (NumberFormatException e)

      { @@ -311,7 +311,7 @@ return this.isLast(); }

      else if (methodName.equals("hashCode"))

      { - return new Integer(System.identityHashCode(proxy)); + return Integer.valueOf(System.identityHashCode(proxy)); }

      else if (methodName.equals("toString")) {
      return "MockResultSet " + System.identityHashCode(proxy);
      Index: src/test/org/apache/commons/dbutils/QueryRunnerTest.java
      ===================================================================
      — src/test/org/apache/commons/dbutils/QueryRunnerTest.java (revision 1025562)
      +++ src/test/org/apache/commons/dbutils/QueryRunnerTest.java (working copy)
      @@ -79,12 +79,12 @@
      public Object invoke(Object proxy, Method method, Object[] args)
      throws Throwable {
      if (getParameterCount.equals(method))

      { - return new Integer(types.length); + return Integer.valueOf(types.length); }

      if (getParameterType.equals(method))

      { if (simulateOracle) throw new SQLException("Oracle fails when you call getParameterType"); int arg = ((Integer)args[0]).intValue(); - return new Integer(types[arg-1]); + return Integer.valueOf(types[arg-1]); }

      return null;
      }
      Index: src/test/org/apache/commons/dbutils/BasicRowProcessorTest.java
      ===================================================================
      — src/test/org/apache/commons/dbutils/BasicRowProcessorTest.java (revision 1025562)
      +++ src/test/org/apache/commons/dbutils/BasicRowProcessorTest.java (working copy)
      @@ -77,7 +77,7 @@
      assertEquals("6", row.getThree());
      assertEquals("not set", row.getDoNotSet());
      assertEquals(3, row.getIntTest());

      • assertEquals(new Integer(4), row.getIntegerTest());
        + assertEquals(Integer.valueOf(4), row.getIntegerTest());
        assertEquals(null, row.getNullObjectTest());
        assertEquals(0, row.getNullPrimitiveTest());
        // test date -> string handling
        @@ -107,7 +107,7 @@
        assertEquals("6", b.getThree());
        assertEquals("not set", b.getDoNotSet());
        assertEquals(3, b.getIntTest());
      • assertEquals(new Integer(4), b.getIntegerTest());
        + assertEquals(Integer.valueOf(4), b.getIntegerTest());
        assertEquals(null, b.getNullObjectTest());
        assertEquals(0, b.getNullPrimitiveTest());
        // test date -> string handling
        Index: src/test/org/apache/commons/dbutils/TestBean.java
        ===================================================================
          • src/test/org/apache/commons/dbutils/TestBean.java (revision 1025562)
            +++ src/test/org/apache/commons/dbutils/TestBean.java (working copy)
            @@ -29,7 +29,7 @@

      private int intTest = 0;

      • private Integer integerTest = new Integer(0);
        + private Integer integerTest = Integer.valueOf(0);

      private String doNotSet = "not set";

      Index: src/test/org/apache/commons/dbutils/MockResultSetMetaData.java
      ===================================================================
      — src/test/org/apache/commons/dbutils/MockResultSetMetaData.java (revision 1025562)
      +++ src/test/org/apache/commons/dbutils/MockResultSetMetaData.java (working copy)
      @@ -64,7 +64,7 @@
      String methodName = method.getName();

      if (methodName.equals("getColumnCount"))

      { - return new Integer(this.columnNames.length); + return Integer.valueOf(this.columnNames.length); }

      else if (
      methodName.equals("getColumnName"))

      { @@ -79,7 +79,7 @@ return this.columnLabels[col]; }

      else if (methodName.equals("hashCode"))

      { - return new Integer(System.identityHashCode(proxy)); + return Integer.valueOf(System.identityHashCode(proxy)); }

      else if (methodName.equals("toString")) {
      return "MockResultSetMetaData " + System.identityHashCode(proxy);
      Index: src/test/org/apache/commons/dbutils/handlers/KeyedHandlerTest.java
      ===================================================================
      — src/test/org/apache/commons/dbutils/handlers/KeyedHandlerTest.java (revision 1025562)
      +++ src/test/org/apache/commons/dbutils/handlers/KeyedHandlerTest.java (working copy)
      @@ -17,8 +17,8 @@
      package org.apache.commons.dbutils.handlers;

      import java.sql.SQLException;
      -import java.util.Iterator;
      import java.util.Map;
      +import java.util.Map.Entry;

      import org.apache.commons.dbutils.BaseTestCase;
      import org.apache.commons.dbutils.ResultSetHandler;
      @@ -33,16 +33,15 @@
      assertNotNull(results);
      assertEquals(ROWS, results.size());

      • Iterator<Object> iter = results.keySet().iterator();
        Map<String,Object> row = null;
      • while (iter.hasNext()) {
      • Object key = iter.next();
      • assertNotNull(key);
      • row = results.get(key);
        + for(Entry<Object, Map<String, Object>> entry : results.entrySet())
        + { + Object key = entry.getKey(); + assertNotNull(key); + row = entry.getValue(); assertNotNull(row); assertEquals(COLS, row.keySet().size()); }
        -
        row = results.get("1");
        assertEquals("1", row.get("one"));
        assertEquals("2", row.get("TWO"));
        @@ -56,16 +55,15 @@
        assertNotNull(results);
        assertEquals(ROWS, results.size());

        - Iterator<Object> iter = results.keySet().iterator();
        Map<String,Object> row = null;
        - while (iter.hasNext()) {
        - Object key = iter.next();
        - assertNotNull(key);
        - row = results.get(key);
        + for(Entry<Object, Map<String, Object>> entry : results.entrySet())
        + { + Object key = entry.getKey(); + assertNotNull(key); + row = entry.getValue(); assertNotNull(row); assertEquals(COLS, row.keySet().size()); }

        -
        row = results.get("5");
        assertEquals("4", row.get("one"));
        assertEquals("5", row.get("TWO"));
        @@ -79,16 +77,15 @@
        assertNotNull(results);
        assertEquals(ROWS, results.size());

      • Iterator<Object> iter = results.keySet().iterator();
        Map<String,Object> row = null;
      • while (iter.hasNext()) {
      • Object key = iter.next();
      • assertNotNull(key);
      • row = results.get(key);
      • assertNotNull(row);
      • assertEquals(COLS, row.keySet().size());
        + for(Entry<Object, Map<String, Object>> entry : results.entrySet())
        + { + Object key = entry.getKey(); + assertNotNull(key); + row = entry.getValue(); + assertNotNull(row); + assertEquals(COLS, row.keySet().size()); }

        -
        row = results.get("6");
        assertEquals("4", row.get("one"));
        assertEquals("5", row.get("TWO"));

        Activity

        Hide
        Henri Yandell added a comment -

        Looks good - thanks for the patch.

        Show
        Henri Yandell added a comment - Looks good - thanks for the patch.
        Hide
        caixiaojian added a comment -

        commons-dbutils.patch

        Show
        caixiaojian added a comment - commons-dbutils.patch

          People

          • Assignee:
            Unassigned
            Reporter:
            caixiaojian
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development