I wish I knew about string.Join a long time ago

Having working on the test team for many products which involve serialization (conversion between CLR objects and various wire formats, such as XML, binary and JSON), I’ve created my share of test types, since there are many, many rules about serialization, and we needed to validate them with actual types. Things such as collections, polymorphism, visibility, decorating attributes, primitive types, classes vs. structures, nested types, enumerations, etc., all needed to be represented in our “type library” so we could write the tests for the serializers. And since we need to understand failures without running the tests under a debugger (since we test in many different platforms, the runs are done in an automated lab, and all we get at the end are logs and pass / rail result), we tend to trace a lot of information, including the actual object contents when there is a mismatch between the actual result and our expectations.

So I’ve written code such as this quite a few times, iterating over the collections in the data, using a counter to find out whether to add a separator, and then adding the items themselves. Nothing out of the ordinary, but I’ve written those 19 lines probably way too much.

  1. public override string ToString()
  2. {
  3.     StringBuilder sb = new StringBuilder();
  4.     sb.Append("Order{Id=");
  5.     sb.Append(this.Id);
  6.     sb.Append(",Items=");
  7.     if (this.Items == null)
  8.     {
  9.         sb.Append("null");
  10.     }
  11.     else
  12.     {
  13.         sb.Append("[");
  14.         for (int i = 0; i < this.Items.Length; i++)
  15.         {
  16.             if (i > 0)
  17.             {
  18.                 sb.Append(",");
  19.             }
  20.  
  21.             sb.Append(this.Items[i]);
  22.         }
  23.  
  24.         sb.Append("]");
  25.     }
  26.  
  27.     sb.Append("}");
  28.     return sb.ToString();
  29. }

Then I saw the light in string.Join, which does exactly what I had been doing “by hand” all that time. Those lines can now be reduced in half – and less code is (almost) always better code, so that’s a gain.

  1. if (this.Items == null)
  2. {
  3.     sb.Append("null");
  4. }
  5. else
  6. {
  7.     sb.Append("[");
  8.     sb.Append(string.Join(",", (IEnumerable<Product>)this.Items));
  9.     sb.Append("]");
  10. }

And if you’re really into reduction, we can go even further, without sacrificing the readability of the code, IMO.

  1. sb.Append(this.Items == null ?
  2.     "null" :
  3.     "[" + string.Join(",", (IEnumerable<Product>)this.Items) + "]");

I know, this can also be refactored into a helper method, which is exactly what I did in many places, although having the concise option is good for new projects. With a helper method, the call becomes a single line:

  1. sb.Append(this.Items.ToPrettyString());

Where ToPrettyString is defined as an extension method in a helper class (updated on 2012/03/29 – added a missing “Select” statement on the original code, without which the code would stack overflow). The OfType<object> call, which seems innocuous, does the conversion between IEnumerable and IEnumerable<T>, for which the Select extension method is defined.

  1. public static class ToStringExtensions
  2. {
  3.     public static string ToPrettyString(this object obj)
  4.     {
  5.         if (obj == null)
  6.         {
  7.             return "null";
  8.         }
  9.         else if (obj is IEnumerable)
  10.         {
  11.             return "[" + string.Join(",", ((IEnumerable)obj).OfType<object>().Select(o => o.ToPrettyString())) + "]";
  12.         }
  13.         else
  14.         {
  15.             return obj.ToString();
  16.         }
  17.     }
  18. }

Every time I look at code I wrote over a year back I see places where I could have done better. That’s always good to find out that, although some mistakes were made, at least we’re still learning…

Update: after seeing some comments about this post on reddit, I have another one with more context for those who are interested.