views:

341

answers:

5

I have two methods that basically converts underlying checkboxes' text or tag as CSV strings.

These two methods

  • GetSelectedTextAsCsv()
  • GetTagAsCsv()

differ only by which property to extract value from SelectedCheckBoxes, which is of type IList<CheckBox>

    public string GetSelectedTextAsCsv()
    {
        var buffer = new StringBuilder();
        foreach (var cb in SelectedCheckBoxes)
        {
            buffer.Append(cb.Text).Append(",");
        }
        return DropLastComma(buffer.ToString());
    }

    public string GetTagAsCsv()
    {
        var buffer = new StringBuilder();
        foreach (var cb in SelectedCheckBoxes)
        {
            buffer.Append(cb.Tag).Append(",");
        }
        return DropLastComma(buffer.ToString());
    }

I was trying to extract a method that returns a Func<T, TResult> but not sure how I can pull that off. My poor attempt was like the following but I cannot figure out how to extract the property portion as shown in the comment within ConvertToCsv()

    public Func<T, string> ConvertToCsv<T>()
    {
        return propertyName =>
        {
            var buffer = new StringBuilder();
            foreach (var checkBox in SelectedCheckBoxes)
            {
                buffer.Append(
                    /* How can you abstract this portion? like following? */ 
                    checkBox.propertyName
                ).Append(",");
            }
            return DropLastComma(buffer.ToString());
        };
    }

If I am on a wrong track, would you please advise me on how I can refactor above code to use a common method?

[UPDATE 1] Here is the combination of both Brian and Jon's answers

    public string ConvertToCsv<T>(Func<CheckBox, T> getValue)
    {
        var stringValues = SelectedCheckBoxes.Select(
            cb => getValue(cb).ToString()).ToArray();
        return string.Join(",", stringValues);
    }

    public string GetSelectedTextAsCsv()
    {
        return ConvertToCsv(cb => cb.Text);
    }

    public string GetTagAsCsv()
    {
        return ConvertToCsv(cb => cb.Tag);
    }

[UPDATE 2] version 2

    public string GetAsCsv<T>(Func<CheckBox, T> getValue)
    {
        return string.Join(",", SelectedCheckBoxes.Select(
            cb => getValue(cb).ToString()).ToArray());
    }

    public string GetSelectedTextAsCsv()
    {
        return GetAsCsv(cb => cb.Text);
    }

    public string GetTagAsCsv()
    {
        return GetAsCsv(cb => 
            cb.Tag == null ? string.Empty : cb.Tag.ToString());
    }

[UPDATE 3] Made the parameter of GetAsCsv() as a closed generic of CheckBox and string

Func<CheckBox, T> to Func<CheckBox, string>.

That allowed me to make GetAsCsv() even simpler and more readable.

private string GetAsCsv(Func<CheckBox, string> getValue)
{
    return string.Join(",", SelectedCheckBoxes.Select(getValue).ToArray());
}
+19  A: 
public string GetAsCsv(Func<CheckBox, string> getValue)
{
    var buffer = new StringBuilder();
    foreach (var cb in SelectedCheckBoxes)
    {
        buffer.Append(getValue(cb)).Append(",");
    }
    return DropLastComma(buffer.ToString());
}

Then:

GetAsCsv(cb => cb.Tag != null ? cb.Tag.ToString() : string.Empty);
GetAsCsv(cb => cb.Text);
Brian Genisio
I am actually using your solution as well as Jon's. Hard to decide...
Sung Meister
w00t! Functional programming :)
Juliet
@Princess: Is it a functional programming approach? I wan't even aware of it...
Sung Meister
Are you sure you mean to use ??
Daniel LeCheminant
@Daniel L: No, ?? would return the tab ojbect, not a string.
Brian Genisio
+Marked as Answer: Jon's answer was great but had to honor the one who has answered the *original* question.
Sung Meister
A: 

Since the two functions are exactly the same except for the getter, that is where you should start: The moving part.

Haven't brushed up my C# yet, but something along the lines of:

    public string GetCsv(Func<string> getter)
    {
        var buffer = new StringBuilder();
        foreach (var cb in SelectedCheckBoxes)
        {
            buffer.Append(getter()).Append(",");
        }
        return DropLastComma(buffer.ToString());
    }

should work. Also, make SelectedCheckBoxes variable?

Daren Thomas
You read the question wrong, SelectedCheckBoxes is variable, the property that is uses on cb is variable.
Samuel
+18  A: 

I'd use string.Join instead:

string tags = string.Join(",", 
                  SelectedCheckBoxes.Select(cb => Convert.ToString(cb.Tag))
                                    .ToArray());
string text = string.Join(",", 
                  SelectedCheckBoxes.Select(cb => cb.Text).ToArray());

Sure, you could put that into a method, but I probably wouldn't bother for just two calls.

If you wanted to though, here's how it would look using Brian's template:

public string GetAsCsv(Func<CheckBox, string> getValue)
{
    string[] array = SelectedCheckBoxes.Select(getValue).ToArray();
    return string.Join(",", array);
}
Jon Skeet
Very nice reduction.
Brian Genisio
Wow. Now that will allow me to completely remove "DropLastComma()"
Sung Meister
Doesn't string.Join expect a string[]? How does this work for Tag, which is an object?
Daniel LeCheminant
... unless I'm completely missing something, I don't think your approach for tags will even compile :-/
Daniel LeCheminant
@Danie L: You are right. I had to modify the source a bit. My solution is actually a combination of both Jon and Brian's answers
Sung Meister
Will fix for stringness...
Jon Skeet
@Sung: Ok ... I suppose it's funny/sad that when I get a compiler error from Jon's code, I assume that I must have done something wrong!
Daniel LeCheminant
@Jon: Won't that throw a NullReferenceException if the Tag is null?
Daniel LeCheminant
@Daniel: Yes, it will. I was assuming that the tags wouldn't be null. Editing...
Jon Skeet
I agree that you don't need to make this a function.
Dan Goldstein
@Dan Goldstein: I like "GetAsCsv()" so much, I will probably place that into a common "String" library in my code base.
Sung Meister
+2  A: 

You could use a lambda:

public string ConvertToCSV(Func<CheckBox, string> cb_prop) {
    ...
    buffer.Append(cb_prop(cb)).Append(",");
    ...

}

ConvertToCSV(c => c.Tag);
Gabe Moothart
+1  A: 

I'd just write a short extension method around IEnumerable string that took a separator:

public static string Join(this IEnumerable<string> strings, string separator)
{
    return string.Join(separator, strings.ToArray());
}

then you can do:

var text = SelectedCheckBoxes.Select(cb => cb.Text).Join(", ");
var tags = SelectedCheckBoxes.Select(cb => (string)cb.Tag).Join(", ");
meandmycode