CODEWITHSUNDEEP
c#if-statement
VasilijZ
VasilijZ

Reputation: 35

Rewriting multiple if statements

I am looking for best practice to write multiple if statements and making code more reusable. these are my if statements that i wuld like to change 

    public DataSet getOrganizationDataSet(string organizationType, string 
    name, string state, string city, string county, string zip)
    {
        string search = "";

        if (organizationType != "")
        {
            search = search + "&type=" + organizationType;
        }
        if (name != "")
        {
            search += "&name=" + name;
        }
        if (city != "")
        {
            search = search + "&town=" + city;
        }
        if (zip != "")
        {
            search = search + "&zip=" + zip;
        }
        if (county != "")
        {
            search = search + "&county=" + county;
        }
        if (state != "")
        {
            search = search + "&state=" + state;
        }
}

i am thinking about writing code like this to make it more readable:

 public DataSet getOrgDataSet(string type, string name, string state, 
 string city, string county, string zip)
    {
        string search = "";

        if ((type ?? state ?? name ?? city ?? county ?? zip) != "") {

            search += "&type=" + type;
            search += "&name=" + name;
            search += "&town=" + city;
            search += "&county=" + county;
            search += "&zip=" + zip;
            search +=  "&state=" + state;

     }

I would like to know your opinion on this and best practices. Thanks in advance, and sorry for novice question, i am still learning c#

Upvotes: 2

Views: 182

Answers (5)

Yurii N.
Yurii N.

Reputation: 5703

The best practice would be to create OrganizationFilter model, with GetSearchString method, which uses dasblinkenlight answer's proposed private method:

public class OrganizationFilter
{
    public string OrganizationType { get; set; }
    public string Name { get; set; }
    public string State { get; set; }
    public string City { get; set; }
    public string Country { get; set; } 
    public string Zip { get; set; }

    public string GetSearchString()
}

Then, you will have your method rewritten like this:

public DataSet getOrganizationDataSet(OrganizationFilter filter)
{
    string search = filter.GetSearchString();
    //...
}

Upvotes: -1

Enigmativity
Enigmativity

Reputation: 117175

I wouldn't know if it is best practice (and asking for that makes your question off-topic, btw), but this is what I do most of the time:

string search =
    String
        .Join("", new[]
        {
            new { key= "type", value = organizationType },
            new { key= "name", value = name },
            new { key= "town", value = city },
            new { key= "zip", value = zip },
            new { key= "county", value = county },
            new { key= "state", value = state },
        }
        .Where(x => !String.IsNullOrEmpty(x.value))
        .Select(x => $"&{x.key}={x.value}"));

Upvotes: 1

Camilo Terevinto
Camilo Terevinto

Reputation: 32072

Instead of hard-coding the possible values in the implementation, move those details to the caller and allow any number of parameters:

public DataSet getOrganizationDataSet(params KeyValuePair<string, string>[] filters)
{
    var builder = new StringBuilder();

    foreach (var filter in filters)
    {
        //c# 6 / VS2015+
        builder.Append($"&{filter.Key}={filter.Value}");
        // VS2013 and lower
        builder.AppendFormat("&{0}={1}", filter.Key, filter.Value);
    }

    string search = builder.ToString();
}

This way, if the names of the filters change, you don't have to change getOrganizationDataSet. You could call this method like:

var dataSet = getOrganizationDataSet(
    new KeyValuePair<string, string>("city", city),
    new KeyValuePair<string, string>("type", type),
    // ... etc
);

Upvotes: 0

Jocelyn Marcotte
Jocelyn Marcotte

Reputation: 110

Do a private method in your class :

private string AddProperty(String property, String value)
{
    if (value != "")
    {
        return $"&{propertie}={value}";
    }
    return "";
}

Then, you can use it like this :

public DataSet getOrgDataSet(string type, string name, string state, string city, string county, string zip)
{
    string search = "";

    search += AddProperty("type", type);
    search += AddProperty("name", name);
    search += AddProperty("town", city);
    search += AddProperty("county", county);
    search += AddProperty("zip", zip);
    search += AddProperty("state", state);
 }

Upvotes: 2

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 727077

Helper methods are among the most basic means of code reuse. Make a helper method that appends to your query conditionally, like this:

private static void AppendSearch(StringBuilder search, string name, string value) {
    if (!string.IsNullOrEmpty(value)) {
        search.Append($"&{name}={value}");
    }
}

Now you can call this method repeatedly for each individual item to construct your search string:

var search = new StringBuilder();
AppendSearch(search, "name", name);
AppendSearch(search, "town", town);
AppendSearch(search, "zip", zip);
...
var searchString = search.ToString();

Note: The implementation uses C# 6 syntax for string interpolation. Use AppendFormat method if you are using an earlier version of the language:

search.AppendFormat("&{0}={1}", name, value);

Upvotes: 4

Related Questions