Join strings from a List<string> while also formating them

Question

The code I have wrote works fine, this inquiry being purely for educational purposes. I want to know how others would do this better and cleaner. I especially hate the way I add the list items to another list before they are joined. There has to be a more efficient way.

I realize an easy way to make this simple would be to store "OU=" and "DC=" in the database with their associated text, but that just feels unseemly to me.

I am building a string for the container argument of the PrincipalContext class for an LDAP call.

The "lst" List<string> contains DataRows of LDAP Organization Units like "Accounts", "Users", etc.

// Get ou list
List<string> lst = db.sda(sql).Rows.OfType<DataRow>().Select(dr => dr.Field<string>("txt")).ToList();
string OU = string.Empty;
List<string> lst = new List<string>();
foreach (string ou in Web.Info.Ldap.ouList)
{
 lst.Add(string.Format("OU={0}", ou));
}
OU = string.Join(",", lst); 

Result:

OU=Users,OU=Accounts,OU=Employees

I do the same thing to a list called dcList that produces the same kind of string:

DC = string.Join(",", lst); 

Result:

DC=severname,DC=another_value,DC=com

to which I join together with OU to get the complete string, like so:

string container = string.Join(",", OU, DC);

End result:

OU=Users,OU=Accounts,OU=Employees,DC=sever,DC=othervalue,DC=com

• mjolka: You're missing a semicolon at the end of the first line of code. Please copy/paste your code exactly as it is, thanks :)
• Nikita B: You should really fix typos in your code. If you want to show us multiple revisions of the same piece of code - you should split those into different code blocks, instead of putting them all together and creating a mess. Otherwise your question might get closed. Notice, that we deal with real code here, which works and compiles, not with pseudo-code.

Answer 1

String.Join can work with IEnumerable<T> so its not necessary to pass list to it. Same goes for foreach: you do not have to call ToList() at the end of your first query, you can loop through initial enumeration. With LINQ you can join strings like that:

//you can call `ToList()` at the end, if you need to cache query results, but you dont have to
var fields = db.sda(sql).Rows.OfType<DataRow>().Select(dr => dr.Field<string>("txt"));
//you can merge this Select with the previous one, if you are not going to re-use `fields` enumeration
var ou = fields.Select(f => "OU=" + f);
var dc = someOtherFields.Select(f => "DC=" + f);
var result = String.Join(",", ou.Concat(dc));

Answer 2

In addition to tinstaafl' answer you can also use the StringBuilder.Length property and turn this into a extension method like

public static String Join(this IEnumerable<String> list, String category, String delimiter)
{
 StringBuilder sb = new StringBuilder();
 foreach (String item in list)
 {
 sb.AppendFormat("{0}={1},", category, item);
 }
 if (list.Any()) { sb.Length -= 1; }
 return sb.ToString();
} 

This can be called like

String ou = Web.Info.Ldap.ouList.Join("OU", ",");
String dc = Web.Info.Ldap.dcList.Join("DC", ",");
String container = String.Concat(ou, ",", dc);

• ANeves: I find the meaning of this method unclear. IMO, readability and comprehension trump speed.

Answer 3

One simpler way to get the formatted strings is to use a StringBuilder and build the string in the loop instead of creating a separate list and joining it:

string MakeFormattedString(List<string> input, string category)
{
 StringBuilder sb = new StringBuilder();
 int i = 0;
 for (; i < input.Count - 1; i++ )
 {
 sb.AppendFormat(category + "={0},", input[i]);
 }
 sb.AppendFormat(category + "={0}", input[i]);
 return sb.ToString();
}

Then something like this should work:

 string OU = MakeFormattedString(Web.Info.Ldap.ouList, "OU");
 string DC = MakeFormattedString(Web.Info.Ldap.dcList, "DC");
 string container = string.Join(",", OU, DC);

• Heslacher: Instead of sb.AppendFormat(category + "={0},", input[i]); you could simply do sb.AppendFormat("{0}={1},",category, input[i]);
• ANeves: -1: function explodes if called with 0 inputs.