Archive for the ‘CodeSOD’ Category

« Older Entries
Newer Entries »

CodeSOD: Starring The Admin

Monday, November 23rd, 2009

We've all been there before. You spend all this time building a kick-ass, ultra-awesome, super-sweet web application and then you realize you need to build some stupid "administration" module that needs to do the boring, run-of-the-mill things like maintain users, groups, privileges, and so on.

There are several different magnitudes of complexity that can be involved with an administration module, ranging from the full-on set of tables including users, groups, roles, tasks, operations, etc., to a simple IsAdmin column on the users table. Actually, it turns out there's an even simpler way, and that Adam P's predecessor discovered and implemented it for their client's fairly large ecommerce website.

if(strstr($username, '**')) {

    $admin = 1;
    $username = str_replace('**', '', $username);
    $_SESSION['admin'] = 1;

} else {

    $admin = 0;

}

That's right: simply add a double-star ("**") to your name when logging in, and you'd have full admin access, including payment records with complete credit card numbers. While you'd think this issue would be Adam's top priority to fix, it turns out that there were even more pressing issues to address, such as being able to enter "/admin/payments" as the URL's path and see those complete payment records without even logging in.


Posted in CodeSOD | Comments Off

CodeSOD: modHmm

Wednesday, November 18th, 2009

"I was put on a new Microsoft Access project recently," Stuart A writes, "and I've slowly been finding my way around the system as the need arises (read: as bugs are reported). As my eyes drifted over the numerous modules, one stopped me in my tracks. It was a module named 'modHmm'. I guessed the programmer was in a ponderous mood?. So naturally, I had a look inside..."

Option Compare Database
Option Explicit

Global t1qq
Global t2qq
Global t3qq
...

"The file started off as usual," Stuart continued, "Globally declared variables without types that were poorly named and to be used who-knows-where. It was pretty standard stuff for our modules. So I continued looking..."

Public Sub UpdateThing()
    Dim adoRst As New ADODB.Recordset
    Dim comRate As Double
   
    adoRst.Open _
        "SELECT * FROM view91 WHERE Transaction_GLAccount_Code = '12055'", _
         Application.CurrentProject.Connection, adOpenStatic, adLockOptimistic, adCmdText
   
    Do Until adoRst.EOF
        adoRst!Transaction_GLAccount_Code = "12056"
        adoRst.Update
        adoRst.MoveNext
    Loop
   
    adoRst.Close
   
End Sub

"Okay. So the function names were turning out to be as descriptive as the module name. Fine. But then I saw..."

Public Sub dude()
    Call sbWriteGLTransaction("2003CD1182", _
          "Cheque Sent Bank", Fdate("2002-07-10"), -4097.47, _
          "Dealer: Diner's Club", glClass.Company_Code, "12025")
    Call sbWriteGLTransaction("2003CD1182", _
          "Cheque Sent Creditor", Fdate("2002-07-10"), 4097.47, _
          "Dealer: Diner's Club", glClass.Company_Code, glClass.GetGLCode("CR"))
End Sub

"Dude?!" Stuart wrote, "not only Dude, but that but Public Dude?! What are we doing?? My eyes continued down the page..."

Public Sub hhhhhfjhskjfds()
    Dim rst As ADODB.Recordset
    Set rst = useADO("View57", 2, , , 1)

    Do Until rst.EOF
        Call ocClass.setDateCompleteOnConsignmentItem( _
          Format(rst!StockMovement_Date, "dd/mm/yyyy"), _
          "Stock Written Off", , _
          rst!OnConsignmentItem_OnConsignment_ID, _
          rst!StockMovement_StockNumber)
        rst.MoveNext
    Loop
End Sub

"I'm stunned!" Stuart added, "the rest of the functions were similarily named: jklsadjflksd(), ggggg(), ddd(), etc."

"Hmmmmm, indeed."


Posted in CodeSOD | Comments Off

CodeSOD: For the Ease of Maintenance

Monday, November 16th, 2009

Ryan Thompson works on a project where all database queries had to go through "stored procedures". Now before you call me out on extraneous quotes or wonder, so what's wrong with stored procedures?, I'm not talking about those kind of stored procedures. I'm talking about "stored procedures" — i.e., the technique developed by Ryan's predecessors for the ease of maintenance.

You see, in Ryan's world, there are hundreds of different procedures, each with a unique identifier such as FNACL0023 or ADUSR0012, all which are stored in a database table named "RawQuery", which, for ease of maintenance, contained the actual SQL queries to be performed. In order to call one of these queries, you'd need to call one of several actual stored procedures — Query0(), Query1(), Query2(), Query4(), Query12(), etc. — depending on the number of "parameters" needed by the query.

So, let's say you wanted to run a query. First, you'd look it up in the RawQuery table as follows.

+-----------+------------------------------------------+
| PROC_ID   |  PROC_QUERY_SQL                          |
+-----------+------------------------------------------+
| FNACL0023 | SELECT ... FROM ... WHERE ACCT_ID = %1%  |
|           |      AND STATUS_CD = '%2%' ORDER BY %3%  |
+-----------+------------------------------------------+

Then, all you had to do was the following:

CALL Query3('FNACL0023','882','ACTIVE','ORDER_DT');

It made for perfectly opaque coding, but the DBA was happy because he could easily update and add queries. Of course, the real problem was that queries were jealously and closely held by the DBA, and despite the "ease of maintenance", it was nearly impossible to convince them to make changes or add new ones.

The DBA ruthlessly audited each new release, forcing the use of stored procedures on every call. Since the manager was a former DBA himself, he always sided with the DBA.

But one day, an enterprising developer discovered a wonderful workaround in the form of query SPTST0001. It was a test query that the DBA used regularly and took only a single parameter: a table name.

+-----------+---------------------+
| PROC_ID   |  PROC_QUERY_SQL     |
+-----------+---------------------+
| SPTST0001 | SELECT * FROM %1%   |
+-----------+---------------------+

Immediately, developers changed their coding style to.

CALL Query3('SPTST0001', '(SELECT a,b,c FROM x,y,z WHERE d=1 AND e='test' ORDER BY z.c)');

All it took was an extra set of parenthesis, and a table name magically became usable as a full SQL query.

The DBA was aware of the workaround, but didn't seem to mind. So long as they were using the stored procedures as required, he didn't care what was actually in them. And besides, the developers finally stopped pestering them to add new queries.


Posted in CodeSOD | Comments Off

CodeSOD: The Long Week

Wednesday, November 11th, 2009

Turns out that I'm The Real WTF, since the Code SOD from earlier today was already posted... last week. Whoops; consequences of posting realllly late at night in a hotel room while at The Business of Software conference I suppose. Anyway, here's one that I'm pretty sure wasn't from last week.

"We recently started using a new CRM system," Gavin Watkinson writes, "and wanted to write some custom functionality for it."

"While looking through the sample code, I came across this snippet. I'm assuming they put this in just on the off chance that the number of days in a week is increased."

Public Function DayOfTheWeek(iDay As Integer) As String
    'Mapping of day number to day-of-the-week string.
    Select Case iDay
    Case 1
        DayOfTheWeek = STR_SUNDAY
    Case 2
        DayOfTheWeek = STR_MONDAY
    Case 3
        DayOfTheWeek = STR_TUESDAY
    Case 4
        DayOfTheWeek = STR_WEDNESDAY
    Case 5
        DayOfTheWeek = STR_THURSDAY
    Case 6
        DayOfTheWeek = STR_FRIDAY
    Case 7
        DayOfTheWeek = STR_SATURDAY
    Case 8
        DayOfTheWeek = STR_SUNDAY
    Case 9
        DayOfTheWeek = STR_MONDAY
    Case 10
        DayOfTheWeek = STR_TUESDAY
    Case 11
        DayOfTheWeek = STR_WEDNESDAY
    Case 12
        DayOfTheWeek = STR_THURSDAY
    Case 13
        DayOfTheWeek = STR_FRIDAY
    End Select
End Function


Posted in CodeSOD | Comments Off

CodeSOD: Modular Process Improvement

Wednesday, November 11th, 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.


Posted in CodeSOD | Comments Off

« Older Entries
Newer Entries »