Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

How to write following if condition clean/fast/readable. Any one of the fields 'address', 'city', 'district', 'province' can be null. I want to make full address by concat all these fields. Like

address = "Street # 32"
city = "My City"
District = "New York"
Province = "NY"

fullAddress = "Stree #32, My City, New York, NY"

If any information is missing then don't add this in string like if district is missing then don't include it.

address = "Street # 32"
city = "My City"
District = ""
Province = "NY"

fullAddress = "Stree #32, My City, NY"

Following code I have written but not good.

string address = dr["Address"].ToString();
string city = dr["City"].ToString();
string district = dr["District"].ToString();
string province = dr["Province"].ToString();
string fullAddress = "";

if (!string.IsNullOrEmpty(address) && !string.IsNullOrEmpty(city) && !string.IsNullOrEmpty(district) && !string.IsNullOrEmpty(province))
{
    fullAddress = address + ", " + city + ", " + district + ", " + province;
}
else if (!string.IsNullOrEmpty(address) && !string.IsNullOrEmpty(city) && !string.IsNullOrEmpty(district))
{
    fullAddress = address + ", " + city + ", " + district;
}
else if (!string.IsNullOrEmpty(address) && !string.IsNullOrEmpty(city) && !string.IsNullOrEmpty(province))
{
    fullAddress = address + ", " + city + ", " + province;
}
else if (!string.IsNullOrEmpty(address) && !string.IsNullOrEmpty(district) && !string.IsNullOrEmpty(province))
{
    fullAddress = address + ", " + district + ", " + province;
}
else if (!string.IsNullOrEmpty(city) && !string.IsNullOrEmpty(district) && !string.IsNullOrEmpty(province))
{
    fullAddress = city + ", " + district + ", " + province;
}
else if (!string.IsNullOrEmpty(address) && !string.IsNullOrEmpty(city))
{
    fullAddress = address + ", " + city;
}
else if (!string.IsNullOrEmpty(address) && !string.IsNullOrEmpty(province))
{
    fullAddress = address + ", " + province;
}
else if (!string.IsNullOrEmpty(address))
{
    fullAddress = address;
}
else if (!string.IsNullOrEmpty(city))
{
    fullAddress = city;
}
share|improve this question
add comment

3 Answers

up vote 12 down vote accepted

I think using LINQ together with string.Join() would be the best option here:

var fields = new[] { "Address", "City", "District", "Province" };
var setValues = fields.Select(field => (string)dr[field])
                      .Where(value => !string.IsNullOrEmpty(value));
string fullAddress = string.Join(", ", setValues);
share|improve this answer
add comment

I'd suggest to create a method like:

public static string CreateAddressFrom(Dictionary dr)
{
   var addressDetails = new List<string>{"Address","City","District","Province"};
   var fullAddress = string.Empty;

   foreach (var addressDetail in addressDetails)
   {
       string addressDetailStr = dr[addressDetail];
       if (!string.IsNullOrEmpty(addressDetailStr))
           fullAddress += fullAddress + ", " + addressDetailStr;
   }

   return fullAddress.TrimStart(", ".ToCharArray());
}

So your client code now can generate the address like this:

...
var myDictionary = ...;
var fullAddress = CreateAddrressFrom(myDictionary);
...

Hope this helps.

share|improve this answer
2  
Why do you use ?? when you then check for null again by using IsNullOrEmpty()? Also, Convert.ToChar(", ") doesn't make sense and will throw an exception. Maybe you meant ", ".ToCharArray()? –  svick Oct 4 '12 at 12:23
    
Yes, my error! I'll edit my answer. –  parajao Oct 4 '12 at 12:37
add comment

I know there's already an accpeted answer here. But I have another solution, a bit different.

Dictionary<string, string> dr = GetValues(); 
//GetValues is where you would get the data

string addressFromDictionary = dr.Where(k => !String.IsNullOrEmpty(k.Value))
                                 .Select(d => d.Value)
                                 .Aggregate((c, n) => c + ", " + n);

svick's solution works fine. But I would avoid using a separate list with the field-keys. And using the Select() before the Where() first gets all fields from the dictionary and then selects the non-empty ones. Here you only get the fields that have a non-null value and concatenate those values using the Aggregate() function.

Not saying my code is better, just showing another possibility! ;)

share|improve this answer
1  
It's okay to add another answer, even if there already is an accepted one. –  svick Feb 13 '13 at 3:01
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.