In Code We Trust



  • (this was submitted to the main site, but in case it's not published, it's here for your amusement)

    A gem from a colleague who quit recently, and he is now working for the state (your tax dollars at work).  Some funny stuff here. It seems subconsciously he was telling himself something, that perhaps his code was not to be trusted, as he checks many things twice. He uses GetDataByPrint, which returns all rows where "Print=True".  Just in case, he then checks for "Print=True".  He also checks if one table contains a key, then double-checks if the same table does NOT have a key.... even though the avlBook and the invBook tables have a one-to-one relationship: the keys are always the same!
    One of our house rules is that we use strongly-typed datasets using Microsoft's dataset designer in Visual Studio.  For some reason, he insisted on manipulating rows and tables the Hard Way (notice the use of the object named "key").  ComponentOne also has some handy functions for looking up data in a grid that would have made his life easier.  Maybe he just didn't trust anybody else's code either.

    private void LoadListBoxBooks()
    {
        InvDBDS.invBookDataTable dtData = m_initechApp.InvDBDS.invBookAdapter.GetSortedData();
        InvDBDS.avlBookDataTable printBkTbl = m_initechApp.InvDBDS.avlBookAdapter.GetDataByPrint(true);
        bookGrid.Initialize(dtData, "Initech Enterprise Manager");

        for (int idx = 1; idx < bookGrid.Rows.Count; idx++)
        {
            object key = bookGrid[idx, 0];
            key = (object)(( InvDBDS.invBookRow[])dtData.Select(string.Format( "Description = '{0}'", key.ToString())))[0].BookID;
            if (printBkTbl.Rows.Contains(key) && (bool)printBkTbl.Rows.Find(key)["Print"] == true )
            {
                bookGrid.Rows[idx].Selected = true;
                if (!printBkTbl.Rows.Contains(key) && idx == 1)  //do not add first default selected row if not in settings
                    continue;
                else
                    this.actBooks.Add((int)key);
            }
        }
    }

    How should the non-paranoid code have looked?

    private void LoadListBoxBooks()
    {
        bookGrid.Initialize(m_initechApp.InvDBDS.invBookAdapter.GetSortedData(), "Initech Enterprise Manager");
        int nRow = bookGrid.Rows.Fixed;
        foreach (InvDBDS.avlBookRow rowAvlBook in m_initechApp.InvDBDS.avlBookAdapter.GetDataByPrint(true))
        {
            //Match a datarow in the table to a datarow in the grid
            nRow = bookGrid.FindRow(rowAvlBook.invBookRow.Description, nRow, bookGrid.Cols["Description"].SafeIndex, false);
            bookGrid.Rows[nRow].Selected = true;
            this.actBooks.Add(rowAvlBook.BookID);
        }
    }

    PS: some variable names and strings have been changed to protect the innocent

     PPS: this ex-colleague is a goldmine for WTF's.  I'll post more when I have time.
     


Log in to reply