Archive for the ‘CodeSOD’ Category

CodeSOD: Reading Comprehension

Monday, November 9th, 2009

"After nearly eight years working as a C++ developer," Rik V writes, "a certain coworker of mine was finally laid off. One of his jobs was to run an application that he wrote which would compare two directories and put any files that had changed into a third. This was a semi-frequent task, and one that he needed to devote quite a bit of time to each month. On his departure, the task fell to me."

"The first time I ran the application, I noticed that it was taking an exorbitant amount of time to complete. After five minutes, it barely scratched the surface of the directories, so I took off for lunch and returned later to see it took over forty minutes to complete. Curious as to how a directory comparison could take so long, I peeked at the code.

long ReadBinaryFile(CString strFile, BYTE** pResult)
{
    BYTE* pBuffer[256];
    BYTE* pResultBuffer = NULL;
    long nLenResultBuffer = 0;

    CFile file;
      
    if(!file.Open(strFile, CFile::modeRead))
    {
        return NULL;
    }
      
    UINT nBytesRead = 256;

    while(nBytesRead)
    {
        nBytesRead = file.Read(pBuffer, 255);

        if(nBytesRead)
        {
            BYTE* pNewBuffer = new BYTE[nBytesRead + nLenResultBuffer];
            ZeroMemory(pNewBuffer, nBytesRead + nLenResultBuffer);
            memcpy(pNewBuffer, pResultBuffer, nLenResultBuffer);
            memcpy(pNewBuffer + nLenResultBuffer, pBuffer, nBytesRead);
            delete[] pResultBuffer;
            pResultBuffer = pNewBuffer;
            nLenResultBuffer += nBytesRead;
        }
    }

    *pResult = pResultBuffer;
    return nLenResultBuffer;
}

"I was stunned," Rik continued, "for a 5MB file, this would involve 20,000+ loops, 20,000+ memory allocations of gradually increasing size, and at least 40,000 memcopy operations. And that's just for one file!"

"Not knowing whether to laugh or cry, I decided to take the safe approach and just rewrite it. Re-starting the app, it finished in under twenty seconds. With a couple of logic tweaks elsewhere in the code, I got this down to thirteen seconds. From fourty minutes.

Rik adds, "I'm not sure if the guy was a moron or a genius. One the one hand, the code is absolutely appalling. On the other, it gave him the opportunity to say 'sorry, the app is still running; I've got to wait for it to finish' and then go back to reading the newspaper.


CodeSOD: Slightly OverSQL’d

Wednesday, November 4th, 2009

"A certain coworker of mine likes to do everything in SQL," Christoffer Hoel writes, "and I mean everything."

"Of course, since our management is non-technical, there's very little any of us can do aside from just toletarting the code. After all, his code runs about the same as ours... and that's all that matters, right?"

// Check if date is valid!
String allowed = "not allowed";

sql = "SELECT IF(DATE(?) <= DATE(?),'allowed','not allowed')";
stmt = con.prepareStatement(sql);
stmt.setString(1, firstDate);
stmt.setString(2, terminationDate);
rs = stmt.executeQuery();

while(rs.next()){
    allowed = rs.getString(1);
}
rs.close();
stmt.close();

if(!allowed.equalsIgnoreCase("allowed")){
    throw new Exception("Termination Date to early!");
}