patterncsharpMinor

Audit table queries for various combinations of search criteria

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

searchcombinationsauditcriteriaforqueriesvarioustable

Problem

I've created a search page on a rather large Audit table (populated by triggers on every database). There are 4 fields you can search by (and I put each textbox next to each other): User ID, Table ID, Date, Record ID. This is for programmer's use and high level managers who would know what these fields mean. Record ID is the important field that list the rows of the unique ID that were either UPDATED, INSERTED, DELETED. Like I mentioned, it's of an Audit table we use to monitor who's doing what on our database.

I display the results using ListView (since I've never used it before and it seems good for this read-only purpose). I've found myself using a ton of else-if's in the search button click. I can't help but think there's a better way. Any suggestions? The last else seems pointless. And the only thing I could put out of scope that would work was ListView1.DataBind(); hence why I repeated a lot of that info in each else if statement.

```
string ID = txtSearch.Text.ToString().Trim();
string lTABLE = ddlTable.SelectedItem.Text.ToString().Trim();
string lRecord = txtRecordID.Text.ToString().Trim();
string lDATE = misc_class.format_mmddyyyy(txtDate.Text).Trim();
string constr2 = "Server=...000...; Database=Audit;uid=junk;pwd=junk;"; // eliminated real info for privacy

if (ID == "" && lTABLE == "" && lDATE == "" && lRecord == "") //ALL 4 blank
{
string query1 = "SELECT TOP 100 [pk],[databaseName],[tableName],[tablefk],[fieldname],[old],[new],[userfk],[action],[entrytime],[username] FROM AUDITACT order by entrytime desc";
SqlDataAdapter da = new SqlDataAdapter(query1, constr2);
DataTable table2 = new DataTable();
da.Fill(table2);
ListView1.DataSource = table2;
}
else if (lRecord != "") // if Record ID has any value entered in it, search by it only, disregard other textboxes
{
string query1 = "SELECT [pk],[databaseName],[tableName],[tablefk],[fieldname],[old],[new],[us

Solution

First of all you should remove duplication. It's clearly that every if..else block has same code for filling DataTable and binding it to ListView. Move it to the bottom:

string query;

if (ID == "" && lTABLE == "" && lDATE == "" && lRecord == "")  //ALL 4 blank
{
    query = "SELECT TOP 100 [pk],[databaseName],[tableName],[tablefk],[fieldname],[old],[new],[userfk],[action],[entrytime],[username] FROM AUDITACT order by entrytime desc";             
}
else if (lRecord != "")
{
    query = "SELECT [pk],[databaseName],[tableName],[tablefk],[fieldname],[old],[new],[userfk],[action],[entrytime],[username] FROM AUDITACT where [tablefk] = '" + lRecord + "' order by entrytime desc";                
}
// ...      
else
{
    query = "SELECT [pk],[databaseName],[tableName],[tablefk],[fieldname],[old],[new],[userfk],[action],[entrytime],[username] FROM AUDITACT where [tablefk] = '" + lRecord + "' order by entrytime desc"; 
}

SqlDataAdapter da = new SqlDataAdapter(query, connectionString);
DataTable table = new DataTable();
da.Fill(table);
ListView1.DataSource = table;
ListView1.DataBind();

Now all your if..else blocks is just a query string generation. But here goes your biggest problem. Your application is vulnerable to SQL Injection attacks (I strongly recommend you to follow link and read article). Instead of concatenating query string you should pass values to SQL server as command parameters. And command building code can look like:

private SqlCommand CreateAuditSelectCommand(string id, string entryDate, string tableName, string record)
{
    var cmd = new SqlCommand();
    List filters = new List();

    if (!String.IsNullOrEmpty(id)) {
        filters.Add("([old] = @ID OR [new] = @ID}");
        cmd.Parameters.AddWithValue("@ID", id);
    }

    if (!String.IsNullOrEmpty(record)) {
        filters.Add("[tablefk] = @Record");
        cmd.Parameters.AddWithValue("@Record", record);
    }

    if (!String.IsNullOrEmpty(entryDate)) {
        filters.Add("CONVERT(date,[entrytime] = @Date");
        cmd.Parameters.AddWithValue("@Date", entryDate);
    }

    if (!String.IsNullOrEmpty(tableName)) {
        filters.Add("[tablename] = @TableName");
        cmd.Parameters.AddWithValue("@TableName", tableName);
    }

    string whereClause = filters.Any() ? "WHERE " + String.Join(" AND ", filters) : "";
    string count = filters.Any() ? "" : "TOP 100";

    var query = String.Format(
            @"SELECT {0} [pk],[databaseName],[tableName],[tablefk],[fieldname],[old],[new],[userfk],[action],[entrytime],[username]
            FROM AUDITACT {1}
            ORDER BY entrytime DESC", count, whereClause);

    cmd.CommandText = query;
    return cmd;
}

Pretty clean. You just check if parameter has value, add it command and add new filter for where clause. After that you just join all filters and you have query string ready.

Next step is separating Data Access code from UI code, and making sure that connection and command are closed after usage. Following method does not know anything about UI. Actually you can move both these methods to separate class (Repository):

private DataTable GetAuditActs(string id, string entryDate, string tableName, string record)
{
    string connectionString = ConfigurationManager.ConnectionStrings["Audit"].ConnectionString;
    using (var conn = new SqlConnection(connectionString))
    using (var cmd = CreateAuditSelectCommand(id, entryDate, tableName, record))
    {
        cmd.Connection = conn;
        conn.Open();
        SqlDataAdapter adapter = new SqlDataAdapter(cmd);
        DataTable table = new DataTable();
        adapter.Fill(table);
        return table;
    }
}

And now you have nice clean method free from any data access specific code:

string id = txtSearch.Text.ToString().Trim();
string tableName = ddlTable.SelectedItem.Text.ToString().Trim();
string record = txtRecordID.Text.ToString().Trim();
string entryDate = misc_class.format_mmddyyyy(txtDate.Text).Trim();

ListView1.DataSource = GetAuditActs(id, entryDate, tableName, record);
ListView1.DataBind();

Code Snippets

string query;

if (ID == "" && lTABLE == "" && lDATE == "" && lRecord == "")  //ALL 4 blank
{
    query = "SELECT TOP 100 [pk],[databaseName],[tableName],[tablefk],[fieldname],[old],[new],[userfk],[action],[entrytime],[username] FROM AUDITACT order by entrytime desc";             
}
else if (lRecord != "")
{
    query = "SELECT [pk],[databaseName],[tableName],[tablefk],[fieldname],[old],[new],[userfk],[action],[entrytime],[username] FROM AUDITACT where [tablefk] = '" + lRecord + "' order by entrytime desc";                
}
// ...      
else
{
    query = "SELECT [pk],[databaseName],[tableName],[tablefk],[fieldname],[old],[new],[userfk],[action],[entrytime],[username] FROM AUDITACT where [tablefk] = '" + lRecord + "' order by entrytime desc"; 
}

SqlDataAdapter da = new SqlDataAdapter(query, connectionString);
DataTable table = new DataTable();
da.Fill(table);
ListView1.DataSource = table;
ListView1.DataBind();

private SqlCommand CreateAuditSelectCommand(string id, string entryDate, string tableName, string record)
{
    var cmd = new SqlCommand();
    List<string> filters = new List<string>();

    if (!String.IsNullOrEmpty(id)) {
        filters.Add("([old] = @ID OR [new] = @ID}");
        cmd.Parameters.AddWithValue("@ID", id);
    }

    if (!String.IsNullOrEmpty(record)) {
        filters.Add("[tablefk] = @Record");
        cmd.Parameters.AddWithValue("@Record", record);
    }

    if (!String.IsNullOrEmpty(entryDate)) {
        filters.Add("CONVERT(date,[entrytime] = @Date");
        cmd.Parameters.AddWithValue("@Date", entryDate);
    }

    if (!String.IsNullOrEmpty(tableName)) {
        filters.Add("[tablename] = @TableName");
        cmd.Parameters.AddWithValue("@TableName", tableName);
    }

    string whereClause = filters.Any() ? "WHERE " + String.Join(" AND ", filters) : "";
    string count = filters.Any() ? "" : "TOP 100";

    var query = String.Format(
            @"SELECT {0} [pk],[databaseName],[tableName],[tablefk],[fieldname],[old],[new],[userfk],[action],[entrytime],[username]
            FROM AUDITACT {1}
            ORDER BY entrytime DESC", count, whereClause);

    cmd.CommandText = query;
    return cmd;
}

private DataTable GetAuditActs(string id, string entryDate, string tableName, string record)
{
    string connectionString = ConfigurationManager.ConnectionStrings["Audit"].ConnectionString;
    using (var conn = new SqlConnection(connectionString))
    using (var cmd = CreateAuditSelectCommand(id, entryDate, tableName, record))
    {
        cmd.Connection = conn;
        conn.Open();
        SqlDataAdapter adapter = new SqlDataAdapter(cmd);
        DataTable table = new DataTable();
        adapter.Fill(table);
        return table;
    }
}

string id = txtSearch.Text.ToString().Trim();
string tableName = ddlTable.SelectedItem.Text.ToString().Trim();
string record = txtRecordID.Text.ToString().Trim();
string entryDate = misc_class.format_mmddyyyy(txtDate.Text).Trim();

ListView1.DataSource = GetAuditActs(id, entryDate, tableName, record);
ListView1.DataBind();

Context

StackExchange Code Review Q#108037, answer score: 7

Revisions (0)

No revisions yet.