views:

127

answers:

6

I have an Address object that has properties AddressLine1, AddressLine2, Suburb, State, ZipCode. (there are more but this is enough for the example). Also, each of these properties are strings. And I'm using C# 3.0.

I'm wanting to represent it as a string but as I'm doing that I'm finding that I'm creating a method with a high cyclomatic complexity due to all the if-statements...

Assuming the string assigned to each property is the same as the name of the property (ie AddressLine1 = "AddressLine1")...I want the address to be represented as follows:

"AddressLine1 AddressLine2 Suburb State ZipCode".

Now, the original way I had done this was by a simple String.Format()

String.Format("{0} {1} {2} {3} {4}", address.AddressLine1, 
address.AddressLine2, address.Suburb, address.State, address.ZipCode);

This is all well and good until I've found that some of these fields can be empty...in particular AddressLine2. The result is additional unneeded spaces...which are particularly annoying when you get several in a row.

To get around this issue, and the only solution I can think of, I've had to build the string manually and only add the address properties to the string if they are not null or empty.

ie

string addressAsString = String.Empty;

if (!String.IsNullOrEmpty(address.AddressLine1))
{
    addressAsString += String.Format("{0}", address.AddressLine1);
}

if(!String.IsNullOrEmpty(address.AddressLine2))
{
    addressAsString += String.Format(" {0}", address.AddressLine2);
}

etc....

Is there a more elegant and/or concise way to achieve this that I'm not thinking about? My solution just feels messy and bloated...but I can't think of a better way to do it...

For all I know, this is my only option given what I want to do...but I just thought I'd throw this out there to see if someone with more experience than me knows of a better way. If theres no better option then oh well...but if there is, then I'll get to learn something I didn't know before.

Thanks in advance!

A: 

First, an address requires certain information, so you should not allow an address that does not have, say, a zip-code. You can also make sure that they are never null by initialize each property to an empty string, or by requiring each property as arguments to the constructor. This way you know that you are dealing with valid data already, so you can just output a formatted string with no need for the IsNullOrEmpty checks.

Ed Swangren
+1  A: 

This possibly isn't the most efficient way, but it is concise. You could put the items you want into an array and filter out the ones without a significant value, then join them.

var items = new[] { line1, line2, suburb, state, ... };
var values = items.Where(s => !string.IsNullOrEmpty(s));
var addr = string.Join(" ", values.ToArray());

Probably more efficient, but somewhat harder to read, would be to aggregate the values into a StringBuilder, e.g.

var items = new[] { line1, line2, suburb, state, ... };
var values = items.Where(s => !string.IsNullOrEmpty(s));
var builder = new StringBuilder(128);
values.Aggregate(builder, (b, s) => b.Append(s).Append(" "));
var addr = builder.ToString(0, builder.Length - 1);

I'd probably lean towards something like the first implementation as it's much simpler and more maintainable code, and then if the performance is a problem consider something more like the second one (if it does turn out to be faster...).

(Note this requires C# 3.0, but you don't mention your language version, so I'm assuming this is OK).

Greg Beech
that looks like an interesting solution...I'll try it out and see how it goes...
mezoid
yup...C# 3.0 is what I'm using...I'll update the tags to reflect that...
mezoid
Awesome solution Greg! Thanks heaps! It reduced 20+ lines to 3 simple to understand lines that will be easy to maintain. I really need to start thinking in Linq.
mezoid
A: 

I had a similar issue building a multi-line contact summary that could have potentially had several empty fields. I pretty much did the same thing you did, except I used a string builder as to not keep concatenating to a string over and over.

Jason
I had considered StringBuilder...but unfortunately the only time using StringBuilder is of any performance benefit is when you have a loop that loops 10 or more times...
mezoid
There are tons of articles and junk science that shows the performance is negligible, but I was referring more to the poor practice of allocating N objects (with +=) when you only need 1 (StringBuilder).
Jason
A: 

If your data isn't completely reliable, you'll need to have that logic somewhere - I'd suggest that you create another read-only property (call it something like FormattedAddress) that does that logic for you. That way, you don't have to change any of the code it, at some point, you clean up or otherwise change the rules.

And +1 for the suggestion to use a stringbuilder, as opposed to concatenating strings.

chris
+1  A: 

When putting strings together I recommend you use the StringBuilder class. Reason for this is that a System.String is immutable, so every change you make to a string simply returns a new string.

If you want to represent an object in text, it might be a good idea to override the ToString() method and put your implementation in there.

Last but not least, with Linq in C# 3.5 you can join those together like Greg Beech just did here, but instead of using string.Join() use:

StringBuilder sb = new StringBuilder();
foreach (var item in values) {
  sb.Append(item);
  sb.Append(" ");
}

Hope this helps.

Jeroen Landheer
+1 for some nice suggestions. :-) I had considered overriding the ToString method but discarded the idea after talking to developers on my project..But doing so would have left me with the same question anyway. As for StringBuilder, there is no performance benefit in my case.
mezoid
+1  A: 

I would recommend overriding the ToString method, and take an IFormatProvider implementation that defines your custom types.

See MSDN at http://msdn.microsoft.com/en-us/library/system.iformatprovider.aspx for information on implementing IFormatProvider.

You could then code such as this:
address.ToString("s"); // short address
address.ToString("whatever"); // whatever custom format you define.

Definitely not the easiest way to do it, but the cleanest IMHO. An example of such implementation is the DateTime class.

Cheers
Ash

Ash M
+1 for the IFormatProvider stuff. I'll consider it for future work...but at the moment I've been instructed by people on my team not to go with the overriding ToString approach so for now I'll just learn about that technique until I'm comfortable and confident in using and maintaining it.
mezoid