How NOT to Code - Example 5

Jan 19, 2010

In case people start missing the point to this, this series is simply a trival game with code related problem.s The goal is to spot the problem(s) in the code. Feel free to comment the answer when you figured it out. Sorry I have no prizes ;) Here is example 5...

<cfscript>
/**
* Returns specified number of random records.
* From http://www.cflib.org/index.cfm?event=page.udfbyid&udfid=524
*/
function QueryRandomRows(theQuery, NumberOfRows) {
    var FinalQuery = QueryNew(theQuery.ColumnList);
    var x = 0;
    var y = 0;
    var i = 0;
    var random_element = 0;
    var random_row = 0;
    var row_list = "";

    if(NumberOfRows GT theQuery.recordcount) NumberOfRows = theQuery.recordcount;

    QueryAddRow(FinalQuery, NumberOfRows);

    // build a list of rows from which we will "scratch off" the randomly selected values in order to avoid repeats
    for (i=1; i LTE theQuery.RecordCount; i=i+1) row_list = row_list & i & ",";

    // Build the new query
    for(x=1; x LTE NumberOfRows; x=x+1){
        // pick a random_row from row_list and delete that element from row_list (to prevent duplicates)
        random_element = RandRange(1, ListLen(row_list)); // pick a random list element
        random_row = ListGetAt(row_list, random_element); // get the corresponding query row number
        row_list = ListDeleteAt(row_list, random_element); // delete the used element from the list
        for(y=1; y LTE ListLen(theQuery.ColumnList); y=y+1) {
            QuerySetCell(FinalQuery, ListGetAt(theQuery.ColumnList, y), theQuery[ListGetAt(theQuery.ColumnList, y)][random_row],x);
        }
    }

    return FinalQuery;
} </cfscript>

Comments

John Mason

John Mason wrote on 01/22/103:15 PM

This one is tricky. There's actually nothing wrong with the code itself. The coders did a good job in that respect. The problem is that they created the function to begin with. Most database engines already provide simple ways to randomize the data for you. Pete Freitag has a great cheat sheet for this at http://www.petefreitag.com/item/466.cfm

This is just a simple case of letting SQL do the work for you.

Write your comment



(it will not be displayed)