How can it get this bad?



  • Integer tempId = new Integer(((Integer) (table.getValueAt(row, column))).intValue());

    - accessing data through the JTable (untyped view) instead of the underlying model (which could be typed, but...)
    - using DefaultTableModel instead of wrapping the data in a custom model
    - assumes that an Integer is stored at that view location (what if someone swaps columns?)
    - creates a new Integer with the original as value because ... defensive copy? (wrong -- Integers are immutable)

     


  • Trolleybus Mechanic

     I want some chocolate, but I don't have any. So you'll have to live with this half-assed attempt at humor:

    WTF wtfWtf = new WTF(((WTF) (forum.getValueAt(row, column))).wtfValue());



  • @Lorne Kates said:

     I want some chocolate, but I don't have any. So you'll have to live with this half-assed attempt at humor:

    WTF wtfWtf = new WTF(((WTF) (forum.getValueAt(row, column))).wtfValue());

     

    Chocolate lornesChocolate = new Chocolate(((Chocolate) (forum.getValueAt(row, column))).chocolateValue()); 

     


  • Considered Harmful

    @mott555 said:

    Chocolate lornesChocolate = new Chocolate(((Chocolate) (forum.getValueAt(row, column))).chocolateValue()); 

    IPerson self = ProgrammerFactory.getProgrammer( "joe" );
    self.Eat( lornesChocolate );
    


  • @mott555 said:

    @Lorne Kates said:

     I want some chocolate, but I don't have any. So you'll have to live with this half-assed attempt at humor:

    WTF wtfWtf = new WTF(((WTF) (forum.getValueAt(row, column))).wtfValue());

     

    Chocolate lornesChocolate = new Chocolate(((Chocolate) (forum.getValueAt(row, column))).chocolateValue());  

    Actually, the correct way to get a new instance of Chocolate is to use a ChocolateFactory.

     



  • private boolean shutdown = false;
    private int connectionRefusalCount = 5;
    private boolean connectionRequestTimeout = false;
    
    public final synchronized int storeNewItems(List<Item> itemCollection) throws SQLException {
    
      int success = -1;
      
      boolean gotCon = false;
      Connection conn = null;
      Statement stmt = null;
      ResultSet rset = 0;
      long gotId = 0L;
      long tmpId = 0L;
    
      while (!gotCon && !shutdown) {
        try { 
          conn = pool.getConnection();
          gotCon = true;
        } catch (OutOfConnectionsException e) {
          System.err.println("No Avail Connections...trying again...1001");
        } catch (Exception e) {
          handleUnanticipatedOccurance("DB_DOWN", e);
          return success;
        }
      }
    
      try {
        stmt = conn.createStatement();
        stmt.setMaxRows(MAX_ROWS);
        stmt.setFetchSize(FETCH_SIZE);
        conn.setAutoCommit(false);
      } catch (SQLException e) {
        conn.close();
        handleUnanticipatedOccurance("DB_DOWN", e);
        return success;
      }
    
    
      for (int ctr = 0; ctr < items.size(); ctr++) {
        Item item = items.get(ctr);
    
        StringBuffer frontSql = new StringBuffer("insert into items (" );
        StringBuffer backSql = new StringBuffer();
    
    /** 
     *  the following lines, 8611-8934 build a dynamic query with string-concatentation 
     *  based on whether values in objects are actually there, instead of creating a 
     *  static/prepared query which puts the actual object field values, null or otherwise
     *  as arguments to the insert query
     *  I'm omitting because ... well ... sanity
     */
    
        try {
          final String newIdQuery = "select item_id_seq.nextval from dual";
          rset = stmt.executeQuery(newIdQuery);
          rset.next();
          gotId = rset.getLong(1);
          tmpId = gotId;
        } catch (SQLException e) {
          try { 
            stmt.close();
            conn.close();
            handleUnanticipatedOccurance("BAD_QUERY", e);
            return success;
          } catch (SQLException e) {
            handleUnanticipatedOccurance("DB_DOWN", e);
            return success;
          }
        }
    
        frontSQL.append(", ITEM_ID");
        backSql.append(", " + gotId);
    
    /** 
     *  the following lines 8959-9247 build the remainder of this query
     */
    
    /**
     *  then, further insertion queries are created for related "sub-query" data
     */
    
        StringBuffer frontSql2 = new StringBuffer("insert into foo (");
        StringBuffer backSql2 = new StringBuffer();
    
    /**
     * lines 9254-9396 build "query 2"
     */
    
        StringBuffer frontSql3 = new StringBuffer("insert into bar (");
        StringBuffer backSql3 = new StringBuffer();
    
    /**
     * lines 9401-9423 build "query 3" (short one, eh?)
     */
    
        try {
          stmt.executeUpdate(frontSql.toString() + ") values (" + backSql.toString() + ")" );
          stmt.executeUpdate(frontSql2.toString() + ") values (" + backSql2.toString() + ")");
          stmt.executeUpdate(frontSql3.toString() + ") values (" + backSql3.toString() + ")");
    
    /**
     *  this is a recent (within the last two years) addition
     */
    
          String supplementInformation = buildSupplementQuery(item);
          if (supplementInformation != null) {
            stmt.executeUpdate(supplementInformation);
          }
    
          conn.commit();
        } catch (SQLException e) {
          try { 
            stmt.close();
            conn.close();
            if (e.toString().indexOf("unique constraint") < 0) {
              handleUnanticipatedOccurance("BAD_QUERY", e);
            }
          return success;
        } catch (SQLException e) {
          handleUnanticipatedOccurance("DB_DOWN", e);
          return success;
        }
    
    /**
     *  now we enter a different style of record insertion
     */
    
        try {
          final String insertQuery = "insert into other (field1, field2, field3) values ( ?, ?, ?)";
          PreparedStatement pstmt = getPreparedStatement(conn, insertQuery);
          pstmt.setLong(1, gotId);
          for (int i = 0; i < item.getSublist().size(); i++) {
            pstmt.setInt(2, i + 1);
            pstmt.setFloat(3, item.getSublist().get(i));
            pstmt.executeUpdate();
          }
          conn.commit();
        } catch (Exception err) {
          try {
            stmt.close();
            conn.close();
            handleUnanticipatedOccurance("BAD_QUERY", e);
            return success;
          } catch (SQLException e) {
            handleUnanticipatedOccurance("DB_DOWN", e);
            return success;
          }
        }
    
    /**
     * yet another style
     */
    
        HashMap dataMap;
        String sep;
        String query;
    
        try {
          dataMap = new HashMap(items.getMapData());
        } catch (NullPointerException e) {
          dataMap = null;
        }
    
        if (dataMap != null) {
          for (int x = 1; x < dataMap.size(); x++) {
            Object o = dataMap.get(new Integer(x));
            if (o != null) {
              sep = "'";
            } else {
              sep = "";
            }
            query = "insert into yet_another_relation ( field1, field2, field3 ) values (" 
                    + gotId + ", " + x + ", " + sep + o.toString() + sep + ")";
    
            try { 
              stmt.executeQuery(query);
            } catch (SQLException e) {
              try {
                stmt.close();
                conn.close();
                handleUnanticipatedOccurance("BAD_QUERY", e);
                return success;
              } catch (SQLException e) {
                handleUnanticipatedOccurance("DB_DOWN", e);
                return success;
              }
            }
          }
        }
    
        if (dataMap != null) {
          dataMap.clear();
        }
      
      } // end loop
    
    /**
     * another couple just like the previous
     * ... now we're up to line 9680
     */
    
      success = (int) tmpId;
      try { 
        conn.commit();
        conn.setAutoCommit(true);
        stmt.close();
        conn.close();
      } catch (SQLException e) {
        handleUnanticipatedOccurance("DB_DOWN", e);
      }
    
      return success;
    
    }
    
    /** 
     * ancillary methods
     */
    
    /**
     * Handles unanticipated events that occur. It should enable the code to recover from
     * database failure or shutdown cleanly if something else occurs
     * @param errorCode <CODE>String</CODE> code indicating if the error is believed to
     * be related to a database problem or a code problem. The values used to indicate
     * this are "BAD_QUERY" and "DB_DOWN"
     * @param e The actual <CODE>Exception</CODE> that was thrown
     */
    
    public void handleUnanticipatedOccurance(String errorCode, Exception e) {
      boolean success;
      if (!aborted) {
        System.err.println(new java.util.Date().toString() + " (" + errorCode + ")";
        e.printStackTrace(System.err);
      }
    }
    
    protected PreparedStatement getPreparedStatement(Connection con, String query) {
      PreparedStatement ps = null;
      if (con == null) {
        con = grabConnection();
      }
      try {
        ps = con.prepareStatement(query);
      } catch (SQLException e) {
        System.out.println("Error in DAO's getPreparedStatement: " + e);
        e.printStackTrace();
        return null;
      }
        return ps;
    } 
    
    public Connection grabConnection() {
      Connection ret = null;
      try {
        ret = getConnection();
      } catch (Exception err) {
        System.out.println("unable to get connection in DAO grabConnection()";
        err.printStackTrace();
      }
      return ret;
    }
    
    public Connection getConnection() throws SQLException, OutOfConnectionsException {
      // initialize variables
      int requestCounter = 0;
      boolean gotCon = false;
      Connection conn = null;
      while (!gotCon && !shutdown) {
        try { 
          conn = pool.getConnection();
          gotCon = true;
        } catch (OutOfConnectionsException ooce) {
          if (requestCounter > connectionRefusalCount) {
            if (connectionRequestTimeout) {        
              throw new OutOfConnectionsException("DAO getConnection() - no connections are currently available");
            }
            System.err.println("DAO getConnection() - no available connections; connection refused");
            return null;
          }
          System.err.println("DAO getConnection() - No Avail Connections...trying again...");
        } catch (SQLException e) {
          handleUnanticipatedOCcurance("DB_DOWN", e);
          throw e;
        }
      }
      return conn;
    }
    


  • Holy shit. I tried to count the WTFs, but it caused an integer overflow. Oddly enough the resulting error just assumed that the database was down.



  • I got this in an old application I'm working on:

     

    IXmlNode mode = (IXmlNode)((IXmlMediumNode)view).Tree.FindByNodeId(someObject.Node);
     

    My delete key's going to need a holiday after today I think.


  • Considered Harmful

    @JimLahey said:

    My delete key's going to need a holiday after today I think.

    I want a Visual Studio add-in or extension that adds an arsenal of weapons you can use to shoot, burn, bomb, and nuke bad code. Each weapon would destroy a different amount of code (gun shoots an identifier, flamethrower burns a line, bomb blows up a method, nuke annihilates a class, etc). Of course it should have vivid animations.



  • @zelmak said:

    /** * the following lines, 8611-8934 build a dynamic query with string-concatentation

    ...

    /** * the following lines 8959-9247 build the remainder of this query */

    frontSql2 = new ...

    backSql2 = new ...

    /** * lines 9254-9396 build "query 2" */

    </p><p>frontSql3 = new ...</p><p>* ... now we're up to line 9680
    

    */

     

    What language is this? OOSpectateJunk?



  • @joe.edwards said:

    I want a Visual Studio add-in or extension that adds an arsenal of weapons you can use to shoot, burn, bomb, and nuke bad code.

    That sounds nice. happy, friendly bombs.

    It might not be enough in our case. I may resort to chucking our SVN server into the brook behind our office building. It's a shame we're landlocked or I'd kick the lot into the sea.



  • @joe.edwards said:

    @JimLahey said:
    My delete key's going to need a holiday after today I think.

    I want a Visual Studio add-in or extension that adds an arsenal of weapons you can use to shoot, burn, bomb, and nuke bad code. Each weapon would destroy a different amount of code (gun shoots an identifier, flamethrower burns a line, bomb blows up a method, nuke annihilates a class, etc). Of course it should have vivid animations.

    Sounds like a great idea for the next edition of Worms. Bonus points if destroying code alters the gameplay accordingly.

     



  • @joe.edwards said:

    I want a Visual Studio add-in or extension that adds an arsenal of weapons you can use to shoot, burn, bomb, and nuke bad code. Each weapon would destroy a different amount of code (gun shoots an identifier, flamethrower burns a line, bomb blows up a method, nuke annihilates a class, etc). Of course it should have vivid animations.

    Seems reasonable


  • Discourse touched me in a no-no place

    @zelmak said:

    Integer tempId = new Integer(((Integer) (table.getValueAt(row, column))).intValue());

    - accessing data through the JTable (untyped view) instead of the underlying model (which could be typed, but...)
    - using DefaultTableModel instead of wrapping the data in a custom model
    - assumes that an Integer is stored at that view location (what if someone swaps columns?)
    - creates a new Integer with the original as value because ... defensive copy? (wrong -- Integers are immutable)

     

     

    The first two issues, while potentially legit, make me want to throw a blakeyrant, but the last two issues are clearly legit, although deeply-nested parens are usually an indicator the person doesn't know why he's getting a compile error.



  • I especially liked this little gem here:

        conn.commit();
        conn.setAutoCommit(true);
        stmt.close();
        conn.close();
    

    So nice of the coder to reset the auto-commit property of the connection before returning it to the pool, so the next piece of code that might get this particular connection from the pool - and, using it under the assumption that it will auto-commit, omits explicit commits - does not end up with lost updates...

    Perhaps it's not the best analogy, but somehow I can't help it but imagining someone desperately trying to rewind a DVD before returning it to the video rental store, because he kind of vaguely seems to remember that that's the Right Thing(tm) to do (without really understanding why).


Log in to reply