0

So I have the following code. If I comment out the line it works as it should but once I uncomment the line Excel wont close properly.

I am unsure what to try next. I stayed away from the 2 dots issue and tried to stick with 1 dot. But now I am unsure what to do now.

dt in the code is a DataTable that I am populating.

Excel.Application app = null;
        Excel.Workbooks books = null;
        Excel.Workbook book = null;
        Excel.Sheets sheets = null;
        Excel.Worksheet sheet = null;
        Excel.Range range = null;

        try
        {
            app = new Excel.Application();
            books = app.Workbooks;
            book = books.Open(FilePath);
            sheets = book.Sheets;
            sheet = book.ActiveSheet;

            int rcount = 45;
            UpdateProgressBarMaxMethod(pbAssets, rcount);

            int i = 0;

            for (; i < rcount; i++)
            {
                //Comment out the below line and it works fine
                dt.Rows.Add(sheet.Cells[i + 1, 1].Value, 
                    sheet.Cells[i + 1, 2].Value, 
                    sheet.Cells[i + 1, 3].Value, 
                    sheet.Cells[i + 1, 4].Value, 
                    sheet.Cells[i + 1, 5].Value, 
                    sheet.Cells[i + 1, 6].Value, 
                    sheet.Cells[i + 1, 7].Value, 
                    sheet.Cells[i + 1, 8].Value, 
                    sheet.Cells[i + 1, 9].Value);

                UpdateProgressBarMethod(pbAssets, i);
            }
            UpdateProgressBarMethod(pbAssets, 0);
            book.Close();
            app.Quit();
        }
        catch (Exception ex)
        {
            MessageBox.Show("Error: Could not read file from disk. Original error: " + ex.Message);
        }
        finally
        {
            if (range != null) System.Runtime.InteropServices.Marshal.ReleaseComObject(range);
            if (sheet != null) System.Runtime.InteropServices.Marshal.ReleaseComObject(sheet);
            if (sheets != null) System.Runtime.InteropServices.Marshal.ReleaseComObject(sheets);
            if (book != null) System.Runtime.InteropServices.Marshal.ReleaseComObject(book);
            if (books != null) System.Runtime.InteropServices.Marshal.ReleaseComObject(books);
            if (app != null) System.Runtime.InteropServices.Marshal.ReleaseComObject(app);
        }
Zach Schulze
  • 304
  • 1
  • 4
  • 17
  • Sorry I will explain more. DT is a DataTable – Zach Schulze May 17 '16 at 21:14
  • I assume you have correctly initialized the datatable. Then, it is the content of the cells (one of them at least) has does not match type of datatable column type. – alsaleem May 17 '16 at 21:16
  • Is it because you're using "2 dots" on `sheet.Cells[i + 1, 2].Value` and therefore not cleaning up the reference to the `Range` returned by `sheet.Cells[i + 1, 2]`? – petelids May 17 '16 at 21:18
  • Yes, I have the DataTable initialized and working properly. The only part that does not work correctly when I run this code is closing excel. But if I comment that line out. Excel runs and closes properly. – Zach Schulze May 17 '16 at 21:18
  • @petelids I didn't even think of that. But not sure what clean up needs done...Since I am I am releasing the object still. What is left? – Zach Schulze May 17 '16 at 21:20
  • Each call to `sheet.Cells[x, y]` creates a `Range` COM object that's not being cleaned up. You are cleaning up `Range` but you never assign anything to it. – petelids May 17 '16 at 21:23
  • I wish people would stop spreading bad advice about using `ReleaseComObject`. Someone somewhere said go try this and everyone copied that example but it is not the correct way to handle it. See [this post from the visual studio team](https://blogs.msdn.microsoft.com/visualstudio/2010/03/01/marshal-releasecomobject-considered-dangerous/). All that really needs to be done is have the GC collect your objects, for proper instructions on how to do that see [this answer](http://stackoverflow.com/questions/17130382/understanding-garbage-collection-in-net/17131389#17131389), – Scott Chamberlain May 17 '16 at 21:37
  • Is it not simply that you have updated the workbook and are closing it without saving it ? In VBA, calling Close() without a False parameter to say not to save it pops up a dialog box asking for the filename. – grahamj42 May 17 '16 at 21:37
  • @Petelids Thanks! didn't even cross my mind. – Zach Schulze May 17 '16 at 21:39
  • @grahamj42 Not an issue since the workbook isn't being updated. Just reading it. – Zach Schulze May 17 '16 at 21:39
  • Sorry, I didn't read correctly about dt. I'm not a C# programmer, but it seems odd that you try to clean up objects in the `finally` block after they should already have been released by `book.Close` and `app.Quit`. – grahamj42 May 17 '16 at 21:51
  • I read the link about using garbage collection and not releasing COM objects. It's worth some serious consideration, but there's still way too much contradictory information, much of it from Microsoft, for it to be conclusive. It shouldn't be that complicated, where you have to rely on barely documented features that even Microsoft can't agree on just to get Excel to open and then quit when expected. That's why I recommend just steering clear of interrop. Libraries like EPPlus do the same work using "normal" documented behaviors and they work predictably. – Scott Hannen May 18 '16 at 01:24

2 Answers2

2

You're not actually following the two dots rule. sheet.Cells[i + 1, 2].Value - that's two dots. It creates a reference to a Range object (the cell).

Obviously creating and cleaning up a Range object for every cell is a pain. This link shows how to create one Range and read the values into an array.

Another option is just not to use Excel.Interop at all because of the huge pain of dealing with all of those COM objects.

Here's a link with some code for EPPlus that will do the same thing without COM objects. There are also some suggestions on using named ranges so that you don't have to deal with all of those row and column numbers.

Community
  • 1
  • 1
Scott Hannen
  • 27,588
  • 3
  • 45
  • 62
  • 2
    I wish people would stop spreading bad advice about using `ReleaseComObject` and the "two dots rule". Someone somewhere said go try this and everyone copied that example but it is not the correct way to handle it. See [this post from the visual studio team](https://blogs.msdn.microsoft.com/visualstudio/2010/03/01/marshal-releasecomobject-considered-dangerous/). All that really needs to be done is have the GC collect your objects, for proper instructions on how to do that see [this answer](http://stackoverflow.com/questions/17130382/understanding-garbage-collection-in-net/17131389#17131389), – Scott Chamberlain May 17 '16 at 21:37
  • Very interesting. In that case there's an amazing ton of bad information out there, much of it on MSDN. I'd worry about it more if I actually had to use `Excel.Interop`. But it seems to me that it only exists because Excel wasn't in XML format years ago. Now that it is, I urge anyone who will listen not to even bother automating Excel. I find it rather annoying that advice which a) came from Microsoft and b) is considered a normal practice is contradicted elsewhere by Microsoft. Not many people will figure out to ignore the MS documentation and heed the *other* MS documentation. – Scott Hannen May 18 '16 at 01:02
  • Although GC.Collect and all was working. I moved away from Excel.Interop. And in speed up the results of loading data 10 fold. Thanks – Zach Schulze May 18 '16 at 15:27
0

Essential XlsIO can import data from Excel to a Datable.

Example code

//The first worksheet object in the worksheets collection is accessed.
IWorksheet sheet = workbook.Worksheets[0];
//Get as DataTable
DataTable datatable = sheet.ExportDataTable(sheet.UsedRange, ExcelExportDataTableOptions.ColumnNames);
//Upload to dataset
DataSet ds = new DataSet();
ds.Tables.Add(datatable);

The whole suite of controls is available for free (commercial applications also) through the community license program if you qualify (less than 1 million US Dollars in revenue). The community license is the full product with no limitations or watermarks.

Note: I work for Syncfusion.

Davis Jebaraj
  • 403
  • 6
  • 10