Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm generating CSV strings for various 3rd party utilities and this section of code gets repeated in many classes. Is there a better way to generate this string?

public override string CsvString()
{
    return (
        string.Format("\u0022{0}\u0022,\u0022{1}\u0022,\u0022{2}\u0022,\u0022{3}\u0022,\u0022{4}\u0022,\u0022{5}\u0022,\u0022{6}\u0022,\u0022{7}\u0022,\u0022{8}\u0022,\u0022{9}\u0022,\u0022{10}\u0022,\u0022{11}\u0022,\u0022{12}\u0022,\u0022{13}\u0022",

        this.BlockType,         //  1, A_NAME
        this.Tag,               //  2, A_TAG
        this.Description,       //  3, A_DESC
        this.InitialScan,       //  4, A_ISCAN
        this.AutoManual,        //  5, A_SCAN
        this.ScanTime,          //  6, A_SCANT
        this.IoDevice,          //  7, A_IODV
        this.IoAddress,         //  8, A_IOAD
        this.InitialAmStatus,   //  9, A_IAM
        this.AlarmPriority,     // 10, A_PRI
        this.AlarmEnable,       // 11, A_ENAB
        this.EnableOutput,      // 12, A_EOUT
        this.HistDescription,   // 13, A_HIST_DESC
        this.SecurityArea1      // 14, A_SECURITYAREA1
    ));
}
share|improve this question
    
I would use a StringBuilder if this was for Java. There might be a C# equivalent? –  Jeremy Heiler Jan 19 '11 at 23:02
    
@Jeremy Heiler, There is :) the exact same structure as well I believe –  RobertPitt Jan 19 '11 at 23:37
add comment

4 Answers 4

up vote 15 down vote accepted

I doubt you'll find a way of not listing all those properties without using reflection, but the following helps to eliminate that huge format string which is likely to become the source of bugs.

var properties = new Object[]
{
    this.BlockType,         //  1, A_NAME
    this.Tag,               //  2, A_TAG
    this.Description,       //  3, A_DESC
    this.InitialScan,       //  4, A_ISCAN
    this.AutoManual,        //  5, A_SCAN
    this.ScanTime,          //  6, A_SCANT
    this.IoDevice,          //  7, A_IODV
    this.IoAddress,         //  8, A_IOAD
    this.InitialAmStatus,   //  9, A_IAM
    this.AlarmPriority,     // 10, A_PRI
    this.AlarmEnable,       // 11, A_ENAB
    this.EnableOutput,      // 12, A_EOUT
    this.HistDescription,   // 13, A_HIST_DESC
    this.SecurityArea1      // 14, A_SECURITYAREA1
}.Select(x => String.Format("\u0022{0}\u0022", x));

return String.Join(",", properties);

A couple of things to note:

This is hardly an efficient way of doing it, but offers fairly maintainable code. If you have an extra property, just add it to the array.

This will only work in .NET 4.0. In earlier versions, you'll have to call ToArray() after that call to Select.

share|improve this answer
    
This is a much cleaner method and addresses the error prone format string. This made my life easier. –  Greg Buehler Jan 20 '11 at 15:15
2  
If you do return "\u0022" + String.Join("\u0022,\u0022", properties) + "\u0022", you can avoid the need for the Select projection altogether. –  ICR Jan 27 '11 at 21:30
add comment

Use a StringBuilder:

sbuilder.AppendFormat("\u0022{0}\u0022,\u0022{1}\u0022,\u0022{2}\u0022,\u0022{3}\u0022,\u0022{4}\u0022,\u0022{5}\u0022,\u0022{6}\u0022,\u0022{7}\u0022,\u0022{8}\u0022,\u0022{9}\u0022,\u0022{10}\u0022,\u0022{11}\u0022,\u0022{12}\u0022,\u0022{13}\u0022",
    this.BlockType,         //  1, A_NAME
    this.Tag,               //  2, A_TAG
    this.Description,       //  3, A_DESC
    this.InitialScan,       //  4, A_ISCAN
    this.AutoManual,        //  5, A_SCAN
    this.ScanTime,          //  6, A_SCANT
    this.IoDevice,          //  7, A_IODV
    this.IoAddress,         //  8, A_IOAD
    this.InitialAmStatus,   //  9, A_IAM
    this.AlarmPriority,     // 10, A_PRI
    this.AlarmEnable,       // 11, A_ENAB
    this.EnableOutput,      // 12, A_EOUT
    this.HistDescription,   // 13, A_HIST_DESC
    this.SecurityArea1      // 14, A_SECURITYAREA1
 ).AppendLine();
share|improve this answer
1  
I wonder if this is what String.Format uses under the hood? –  Jeremy Heiler Jan 19 '11 at 23:40
1  
Actually string.Format uses StringBuilder inside it (or so Reflector says), so there wouldn't memory/performance benefit in this specific case (one shot formatting of the string) –  Jaime Jan 20 '11 at 1:36
add comment

Maybe something like this:

public static string MakeCsvLine(params string[] items)
{
  return String.Format("\u0022{0}\u0022",String.Join("\u0022,\u0022",items));
}

Edit:

On thinking about it might be better to use it to build up a string builder so:

public static void AddCsvLine(StringBuilder sb, params string[] items)
{
  sb.AppendFormat("\u0022{0}\u0022",String.Join("\u0022,\u0022",items))
        .AppendLine();
}

This would remove having to have long strings repeated all over the code. Edit: Made the funtion return the orginal result.

share|improve this answer
1  
You'd have to join on "\u0022,\u0022" and then also add "\u0022" at the beginning and end to get the same result as the original. –  sepp2k Jan 20 '11 at 4:40
    
@sepp2k Your right, I change the code to reflect that –  Sean Lynch Jan 20 '11 at 11:59
add comment

As a variant of Alex Humphrey's solution, You could try this for improved performance:

var properties = new Object[]
{
    this.BlockType,         //  1, A_NAME
    this.Tag,               //  2, A_TAG
    this.Description,       //  3, A_DESC
    this.InitialScan,       //  4, A_ISCAN
    this.AutoManual,        //  5, A_SCAN
    this.ScanTime,          //  6, A_SCANT
    this.IoDevice,          //  7, A_IODV
    this.IoAddress,         //  8, A_IOAD
    this.InitialAmStatus,   //  9, A_IAM
    this.AlarmPriority,     // 10, A_PRI
    this.AlarmEnable,       // 11, A_ENAB
    this.EnableOutput,      // 12, A_EOUT
    this.HistDescription,   // 13, A_HIST_DESC
    this.SecurityArea1      // 14, A_SECURITYAREA1
};

var builder = new StringBuilder(properties.Length * 6);
foreach (var property in properties)
{
    builder.Append('"').Append(property).Append('"').Append(',');
}
builder.Remove(builder.Length - 1, 1); // remove the last comma

return builder.ToString();

But beware that this code is susceptible for failure if any of the properties contains a double quote. You should make sure they are escaped.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.