Parsing Excel file into CSV file EDIT: Thanks everyone for your answers, this is moving quickly now and it's always essential to get better with design and best practice!

I am loading in an excel file to a dataset using ExcelDataReader. I then need to create a *.csv file with the data.

Currently for the excel sheet I'm working with I have about 40 columns and 30k rows of data. The approach I'm taking takes about 2 minutes to complete. 

The second snippet is the offender.

public static Stream SaveAsCsv(Stream excelFile, string filename)
{
    MemoryStream newCSV = new MemoryStream();

    string test = excelFile.ToString();

    IExcelDataReader reader = null;
    if (filename.EndsWith(".xls"))
    {
        reader = ExcelReaderFactory.CreateBinaryReader(excelFile);
    }
    else if (filename.EndsWith(".xlsx"))
    {
        reader = ExcelReaderFactory.CreateOpenXmlReader(excelFile);
    }

    //Read was empty so return a null stream in return
    if (reader == null)
        return newCSV;

    var headers = new List&lt;string&gt;();

    var ds = reader.AsDataSet(new ExcelDataSetConfiguration()
    {
        ConfigureDataTable = (tableReader) =&gt; new ExcelDataTableConfiguration()
        {
            UseHeaderRow = true,

            ReadHeaderRow = rowReader =&gt;
            {
                for (var i = 0; i &lt; rowReader.FieldCount; i++)
                    headers.Add(Convert.ToString(rowReader.GetValue(i)));
            },

            FilterColumn = (columnReader, columnIndex) =&gt;
            !headers[columnIndex].ToString().ToUpper().Contains("SKIP")
        }
    });
    var csvContent = string.Empty;
    int colCount = ds.Tables[0].Columns.Count;


This for operation is what takes two minutes to complete, everything else is not a problem:

    for (int row_no = 0; row_no &lt; ds.Tables[0].Rows.Count; row_no++)
    {
        var arr = new List&lt;string&gt;();
        if (row_no == 0)
        {
            for (int i = 0; i &lt; colCount; i++)
            {
                arr.Add(ds.Tables[0].Columns[i].ColumnName);
            }
        }
        else
        {
            object[] objarr = ds.Tables[0].Rows[row_no].ItemArray;

            arr = ((IEnumerable)objarr).Cast&lt;object&gt;()
                         .Select(x =&gt; x.ToString())
                         .ToList();
        }
        csvContent += string.Join("|", arr) + "\n";
    }


    StreamWriter csv = new StreamWriter(newCSV);
    csv.Write(csvContent);
    csv.Flush();
    csv.BaseStream.Seek(0, SeekOrigin.Begin);

    return csv.BaseStream;
}
You should definitely follow @Henrik Hansen's suggestion about separating your method into multiple APIs with one exception.

The save method should just save the stream. There is no need for it return any streams. You should already know this stream from the CreateCsv method.



Other things that you can improve are...



String concatenation


csvContent += string.Join("|", arr) + "\n";



I'm pretty sure this is the bottleneck of you application. Strings are immutable so in order to concatenate strings, the runtime has to copy the old one and appaned something new to it. Usually this is not a big issue but since you have about 40 columns and 30k rows of data this is really a lot of copying. And the string is growing, so each copy operation has to handle larger and larger strings.

For frequent string manipulation, especially in loops, you should use the StringBuilder that does not requrie copying. There are numerous pages about this topic so if you are interested in its internals, just google for it.



As far as strings are concerned there is one more piece of code that isn't efficient yet.


arr = ((IEnumerable)objarr).Cast&lt;object&gt;()
             .Select(x =&gt; x.ToString())
             .ToList();



You're materializing the objarr by calling ToList. This is not necessary at this point.


string.Join("|", arr)



Join can take of enumerating it later only once. Currently your string is being created twice. 

The new code could look this:

var csvBuilder = new StringBuilder();

for (int row_no = 0; row_no &lt; ds.Tables[0].Rows.Count; row_no++)
{
    if (row_no == 0)
    {
        var headers = new List&lt;string&gt;();
        for (int i = 0; i &lt; colCount; i++)
        {
            headers.Add(ds.Tables[0].Columns[i].ColumnName);
        }
        csvBuilder.AppendLine(string.Join("|", headers));
    }
    else
    {
        var items = ds.Tables[0].Rows[row_no].ItemArray;
        var values = ((IEnumerable)items).Cast&lt;object&gt;().Select(x =&gt; x.ToString());
        csvBuilder.AppendLine(string.Join("|", values));
    }
}




Always dispose streams

Another bad habit of yours is to not dispose streams. If you call this method a couple of time you'll be wasting a lot of memory. You should always do it at some point either with the using statement or by calling Dispose() in a try/finally block.



Naming

You should pay more attention to your variable names. arr is a terrible name because it can stand for just anything. It harms code readability. Code should document itself. This one doesn't.

You should also be more consistent. If you name most of your variables correctly with camelCase then don't name others with _snake_case_ like row_no.

Use always strong names and as precise as you can. Not too long (if possible) but also not to short, avoid abbreviations. columnIndex is a good example. row_no should be rowIndex. Currently it looks like more then one person was writing this code. Each one with different coding style.



{}


for (var i = 0; i &lt; rowReader.FieldCount; i++)
               headers.Add(Convert.ToString(rowReader.GetValue(i)));



Always use {}. You can save youself hours of debugging because without them it's very easy to make a mistake.



Explicit types vs var

You mix explit types and var. You use them interchangeably. This makes your code look very unprovessional and messy. Pick one and stick to it. (I suggest picking var).