views:

1188

answers:

7

In C#, I'm trying to build an extension method for StringBuilder called AppendCollection() that would let me do this:

var sb1 = new StringBuilder();
var sb2 = new StringBuilder();
var people = new List<Person>() { ...init people here... };
var orders = new List<Orders>() { ...init orders here... };

sb1.AppendCollection(people, p => p.ToString());
sb2.AppendCollection(orders, o => o.ToString());

string stringPeople = sb1.ToString();
string stringOrders = sb2.ToString();

stringPeople would end up with a line for each person in the list. Each line would be the result of p.ToString(). Likewise for stringOrders. I'm not quite sure how to write the code to make the lambdas work with generics.

+2  A: 

What is this method suppose to return? I can see a string, but why, if you are appending to a StringBuilder?

What you are trying to do is rather easy, but you need to explain exactly what you want.

Update:

Here's my take. Using an extension method for this is stupid and pointless if you are just going to pass in a new StringBuilder and return a string.

Update 2:

Now that I see that usage, what you are doing is bad practice. What you should ideally be doing is something like:

public static string Print<T>(this IEnumerable<T> col, Func<T,string> printer)
{
  var sb = new StringBuilder();
  foreach (T t in col)
  {
    sb.AppendLine(printer(t));
  }
  return sb.ToString();
}

string[] col = { "Foo" , "Bar" };
string lines = col.Print( s => s);

Update 3:

After more clarification:

public static void AppendCollection<T>(this StringBuilder sb, 
   List<T> col, Func<T,string> printer)
{
  col.ForEach( o => sb.AppendLine(printer(o)));
}

(which is the same as bruno conde said)

And now you dont really need it anymore :)

leppie
So I am getting voted down, because I am asking for clarification? That's the spirit...
leppie
I think the point is that your post isn't an answer, its a question. I did not vote you down.
Jason Jackson
So I was speaking to myself?
leppie
You can avoid the down votes by asking for clarification in a comment rather than a answer. I also did not down vote. That's what I do to avoid people who think that anything that's not an answer is not helpful.
tvanfosson
But I did not want that. I was going to post an answer once I have clarification...
leppie
The method should return void rather than a string like I had at first. It should operate like AppendLine(), but allow you to pass a collection and formatter function.
Lance Fisher
I would like to have an extension method to StringBuilder because there are other strings I want to add to the builder before and after my collection. Why do you think that is bad practice?
Lance Fisher
Up-voted to counter the initial down-votes.
Jason Jackson
Well, you didnt explain the usage, hence why I had so many initial questions.
leppie
Just to be clear I didn't vote you down in the first place, though I can totally see why some people might have.
Jason Jackson
+8  A: 

Use the Func<T,string> delegate.

public static void AppendCollection<T>(this StringBuilder sb, 
                                       IEnumerable<T> collection, Func<T, string> method) {
   foreach(T x in collection) 
       sb.AppendLine(method(x));
}
Mehrdad Afshari
I don't like this as it breaks the paradigm of StringBuilder. Methods on StringBuilder should just continue to add onto the internal buffer until ToString is called on the builder. This combines the append/tostring steps and is not like the other append methods on StringBuilder.
tvanfosson
Sure, I updated the answer to mention my opinion on this, but it's specifically asked in the question.
Mehrdad Afshari
I agree completely. I typed up that example code a little too fast. I've updated the question.
Lance Fisher
I'll update the answer to reflect your change
Mehrdad Afshari
+3  A: 

I'm not sure you need to work that hard:

 public static void AppendCollection( this StringBuilder builder,
                                      ICollection collection )
 {
     foreach (var item in collection)
     {
        builder.AppendLine( Convert.ToString( item ) );
     }
 }

Used as

 List<Person> people = ...

 StringBuilder builder = new StringBuilder();
 builder.AppendCollection( people );
 var s = builder.ToString();

Of course, Person needs to override ToString() to produce the correct output for a Person object.

tvanfosson
By using the lambda, you get to format the item in the collection how ever you want.
Lance Fisher
Sure, but you're just calling ToString()
tvanfosson
I probably should have wrote something like sb1.AppendCollection(p.FirstName + " " + p.LastName) in the example. That's the flexibility I like in this function.
Lance Fisher
Lance, in that case I would rather provide an overload for `AppendCollection` that lets you specify a formatting, and then write a custom formatter for your type. That way, you use established interfaces and facilitate code reuse, instead of creating a whole new interface-breaking API.
Konrad Rudolph
+3  A: 

Something like:

  public static void AppendCollection<TItem>(this StringBuilder builder, IEnumerable<TItem> items, Func<TItem, string> valueSelector)
  {
       foreach(TItem item in items)
       {  
            builder.Append(valueSelector(item));
       }
  }

I would add in a useful default to save specifiying the lambda in 90% of cases...

   public static void AppendCollection<TItem>(this StringBuilder builder, IEnumerable<TItem> items)
  {
      AppendCollection(builder, items, x=>x.ToString());
   }
Jennifer
+2  A: 
static class SBExtention
{
  static string AppendCollection<T>(this StringBuilder sb, 
                                    IEnumerable<T> coll, 
                                    Func<T,string> action)
  {
       foreach(T t in coll)
       {
          sb.Append(action(t));
          sb.Append("\n");
       }
       return sb.ToString();

  }
}

However, I think you'll be better off having it return the StringBuilder. That way you could chain it:

  static StringBuilder AppendCollection<T>(this StringBuilder sb, 
                                    IEnumerable<T> coll, 
                                    Func<T,string> action)
  {
       // same
       return sb;

  }

string peopleAndOrders = sb.AppendCollection(people, p => p.ToString()) .AppendCollection(orders, o => o.ToString()).ToString();

And I agree with Jennifer about the default case:

   public static StringBuilder AppendCollection<TItem>(
                  this StringBuilder builder, 
                  IEnumerable<TItem> items)
  {
      return AppendCollection(builder, items, x=>x.ToString());
   }

string peopleAndOrders = sb.AppendCollection(people).AppendCollection(orders).ToString();

James Curran
The chaining thing is nice, but I generally prefer my extension methods to basically work the same way as the other methods on the class. Changing the basic pattern of how it works makes it harder to understand.
tvanfosson
+4  A: 
 public static void AppendCollection<T>(this StringBuilder builder, IEnumerable<T> list, Func<T,string> func)
        {
            foreach (var item in list)
            {
                builder.AppendLine(func(item));
            }
        }

I wouldn't return a string, I would just append it to the original Stringbuilder that was passed in.

BFree
Yeah, I agree. That was a mistake. I updated the question.
Lance Fisher
+3  A: 

My version:

    public static string AppendCollection<T>(this StringBuilder sb, IEnumerable<T> enumerable, Func<T, string> method)
    {
        List<T> l = new List<T>(enumerable);
        l.ForEach(item => sb.AppendLine(method(item)));
        return sb.ToString();
    }

but you shouldn't return a string in this case. I would prefer the following:

    public static void AppendCollection<T>(this StringBuilder sb, IEnumerable<T> enumerable, Func<T, string> method)
    {
        List<T> l = new List<T>(enumerable);
        l.ForEach(item => sb.AppendLine(method(item)));
    }

to be used like:

        sb.AppendCollection(people, p => p.ToString());
        sb.AppendCollection(orders, o => o.ToString());
        Console.WriteLine(sb.ToString());
bruno conde
I agree that I shouldn't return a string. I've updated the question.
Lance Fisher