I’ve been working on a data repository that’s take bits of an object model and is calling existing SPROCs. Part of the mapping process is this block of code (a common pattern you’ll see many times, and possibly in many different forms):

// Build address string
string address = "";

if (cai.Address1 != null) { address += cai.Address1 + ", " + Environment.NewLine; }
if (cai.Address2 != null) { address += cai.Address2 + ", " + Environment.NewLine; }
if (cai.County != null) { address += cai.County + ", " + Environment.NewLine; }
if (cai.Town != null) { address += cai.Town + ", " + Environment.NewLine; }

address = address.Trim();
if (address.Length > 0 && address.LastIndexOf(',') == address.Length - 1)
{
	address = address.Substring(0, address.Length - 1);
}

A variation of this would be :

var builder = new StringBuilder();
foreach(var thing in listOfThings)
{
	if (thing != null)
	{
		builder.Append(thing.ToString());
		builder.Append(", ");
		builder.Append(Environment.NewLine);
	}
}
var stringOfThings = builder.ToString().Trim();
if (stringOfThings.Length > 0 && stringOfThings.LastIndexOf(',') == stringOfThings.Length - 1)
{
	stringOfThings = stringOfThings.Substring(0, stringOfThings.Length - 1);
}

This pattern is so common it’s not really funny. Basically we need to append a bunch o’ strings together with a seperator (if they’re not null) and make sure there’s not a dangling comma. So far, so relatively ordinary. But look at that code again. The rule, ‘value must not be null’, is repeated 4 times, as is the rule, ‘seperated by ", " + Environment.NewLine’. Also if you wanted to add another line into the address you’ve got to repeat both rules again. This is a violation of the DRY principle (Don’t Repeat Yourself). Now you can extract the ", " + Environment.NewLine out into a variable, but you’ve still repeated the null check. Worse you’ve had to include a particularly error prone bit of code at the end to clean off the hanging comma (the common error you can get here is an off by one). Consider the following replacement:

var validParts = new[] { address.Line1, address.Line2, address.Town, address.County }.Where(p => p != null);

return string.Join(", " + Environment.NewLine, validParts);

And for our variation:

var validParts = listOfThings.Where(t => t != null);

return string.Join(", " + Environment.NewLine, validParts);

Now doesn’t that make you feel better? Stick all our elements into an array, filter out null ones, and then join them with our seperator. Each of our pair of rules is implemented only once (the condition in Where(p => p != null), should really be !string.IsNullOrEmpty(p) but I’m making sure to stick to the existing data contract.) The string.Join method is only putting the seperator in between each element, not after the last one so no need for the error prone clean up. We could go one step further and write it as

return string.Join(", " + Environment.NewLine, new[] { address.Line1, address.Line2, address.Town, address.County }.Where(p => p != null));

But I think the intent gets slightly lost, which is why I kept the former. However both conform to DRY, each rule is stated once and ONLY once. Change is easy to accomodate and we’ve deliberately avoided writing error-prone code.