views:

216

answers:

4

Hi,

I have a database table named call with columns call_time, location, emergency_type and there are three types of emergency: paramedics, police and firefighters. In the windows form I created CheckBoxes 'paramedics', 'police', 'firefighters' and I want to retrieve all table columns which meet user's selection.

I created a function:

public static DataTable GetHistory(DateTime from, DateTime to, bool paramedics, bool police, bool firefighters)
    {
        string select =
            "SELECT call_time, location, emergency_type where call_time between @from AND @to AND";
        if(paramedics)
        {
            select += " emergency_type = 'paramedics' ";
        }
        if(paramedics && police)
        {
           select +=" emergency_type = 'paramedics' OR emergency_type = 'police';
        }
        ...

    }

This code however seems very dirty because if there were 30 kinds of emergency there would be 30! combinations and I would get old before writing all if statements.

I would appreciate if you shared your practice for retrieving data that meet the selected search conditions if there are many options you can chosse.

Thanks!

A: 

This is a dirty way of doing this.

string select = "SELECT call_time, location, emergency_type where call_time between @from AND @to AND (1=0";

if(paramedics) { select += " OR emergency_type = 'paramedics' "; }
if(police)     { select += " OR emergency_type = 'police'"; }
if(xyz)        { select += " OR emergency_type = 'xyz'"; }

select += ")";
Ady
+4  A: 

Well if you have to use emergency_type as a string then instead of passing in bools you could send in a List containing the text representation of the emergency type. For example to adjust the above code you could change the method signature to

public static DataTable GetHistory(DateTime from, DateTime to, List<string> types)
{
 ..
}

and then pass in a list that looked like these (for example)

List<string> types = 
  new List<string> { "paramedics" };

or 

List<string> types = 
  new List<string> { "paramedics", "police" };

Then you could adapt your query to use the SQL IN statement in your where clause. Next convert the list of strings into a comma separated string like

string values = "'paramedics', 'police'"

A simple way to create the values variable is to use

string values = string.Empty;
            types.ForEach(s =>
            {
               if (!string.IsNullOrEmpty(values))
                   values += ",";
               values += string.Format("'{0}'", s);

            });

By the way you could use a parameterized command to avoid SQL injection. Once you have the string you can simply do

string select =
 "SELECT call_time, location, emergency_type where call_time between @from AND @to AND emergency_type IN " + values
TheCodeJunkie
I quite like this approach, assuming that all the conditions are on the same column.
Ady
I did this way and it works fine. Thanks for the suggestion!
niko
A: 

String concatenation should be avoided, as it can contribute to some nasty vulnerabilities. If you're looking for best practices in terms of programmatic access, then the best practice here is to use a parameterized query.

If you want to be cheap, then make the in clause take a parameter, and concatenate that string together from the list of checked checkboxes, and pass that as the value of the parameter for the in clause. it would look like this:

where ... and emergency_type in (?)

The other way to do it is to count the number of checkboxes that are checked, and build the list of parameters to the in clause, so that it looks more like this:

where ... and emergency_type in(?,?...) -- as many params as there are checked checkboxes.

Either of these will do just fine. With these types of queries, i've gone so far as to build my own SQL constructor methods, I keep an internal count of parameters, and their datatypes, and dynamically build the sql, then do the prepare with the known list of good parameters.

You might look at learning Linq.

A: 

Build the user's list of compare values (@EmergencyList) and use SQL with a parameterized query using the Contains operator.

SELECT call_time, location, emergency_type where call_time between @from AND @to AND CONTAINS( Emegency_Type, @EmergencyList )

rp