Extension method to format properties that are not null and not empty

Question

I'm trying to work with the following class, which is inside a library I currently cannot change:

// This class represents "external" code as far as my review
// request is concerned: I cannot change this, currently.
public class AddressDto
{
 public string Name { get; set; }
 public string Zip { get; set; }
 public string City { get; set; }
 public string AddressLine { get; set; }
 public string ExtraInfo { get; set; }
}

I'm writing an extension method with c# 5 and .NET 4.5.1 to represent instances of that class as a string for display purposes. I'm looking to get feedback on the inner workings of this method:

// Looking for feedback on this method:
public static string FormatForGui(this AddressDto Address)
{
 if (Address == null) throw new ArgumentNullException("Address");
 var parts = new[] 
 {
 Address.Name,
 Address.AddressLine,
 Address.Zip,
 Address.City
 };
 return string.Join(" ", parts.Where(s => !String.IsNullOrEmpty(s)));
}

Can this method body be improved?

---

For your convenience, here's a condensed NUnit unit test to define the behavior:

// Not necessarily looking for feedback on these tests, my actual tests are more DAMP 
// but for brevity I condensed them into one test:
[Test]
public void AllTests()
{
 Assert.Throws<ArgumentNullException>(() => ((AddressDto)null).FormatForGui());
 Assert.That(new AddressDto { }.FormatForGui(), Is.EqualTo(""));
 Assert.That(new AddressDto { Name = "" }.FormatForGui(), Is.EqualTo(""));
 Assert.That(new AddressDto { Name = "A" }.FormatForGui(), Is.EqualTo("A"));
 Assert.That(new AddressDto { Name = "A", AddressLine = "B" }.FormatForGui(), Is.EqualTo("A B"));
 Assert.That(new AddressDto { Name = "A", AddressLine = "B", Zip = "C" }.FormatForGui(), Is.EqualTo("A B C"));
 Assert.That(new AddressDto { Name = "A", AddressLine = "B", Zip = "C", City = "D" }.FormatForGui(), Is.EqualTo("A B C D"));
 Assert.That(new AddressDto { Name = "A", AddressLine = "B", City = "D" }.FormatForGui(), Is.EqualTo("A B D"));
}

• svick: The method is short and looks clear to me. What kind of improvement are you looking for?
• Heslacher: Not worth an answer, but the methods parameter should be named using camelCase casing (based on the NET naming guidelines).

Answer 1

You're asking for a review of a tiny amount of decent code, so there is not much to say.

---

return string.Join(" ", parts.Where(s => !String.IsNullOrEmpty(s)));

You're first using string and then String to call a static method. You should be consistent in which one you use.

---

throw new ArgumentNullException("Address");

If you're using C# 6.0, you could use nameof here:

throw new ArgumentNullException(nameof(Address));

(And Heslacher already mentioned in a comment that it should be address.)

---

Assert.That(new AddressDto { }.FormatForGui(), Is.EqualTo(""));

I think that Assert.AreEqual() is better, mostly since it's slightly shorter and doesn't use the weird Is pattern:

Assert.AreEqual("", new AddressDto { }.FormatForGui());