2
votes

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!

7

7 Answers

4
votes

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).

1
votes

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.

1
votes

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

0
votes

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.

0
votes

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.

0
votes

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.

0
votes

I know this is really old, but I sensed a general solution and implemented mine this way:

private static string GetFormattedAddress(
    string address1,
    string address2,
    string city,
    string state,
    string zip)
{
    var addressItems =
        new []
        {
            new[] { address1, "\n" },
            new[] { address2, "\n" },
            new[] { city, ", " },
            new[] { state, " " },
            new[] { zip, null }
        };

    string suffix = null;
    var sb = new StringBuilder(128);

    foreach (var item in addressItems)
    {
        if (!string.IsNullOrWhiteSpace(item[0]))
        {
            // Append the last item's suffix
            sb.Append(suffix);

            // Append the address component
            sb.Append(item[0]);

            // Cache the suffix
            suffix = item[1];
        }
    }

    return sb.ToString();
}