0

I have a function basically to convert an excel file to text file. The structure is like following. What I am concerned is the Finally part? Is it a best practise to kill the excel?

Excel.Application excel=new Excel.Application();
Excel.Workbook wb=excel.Workbooks.Open(…);

Try 
{
   …
   Foreach(Excel.Worksheet sheet in wb.Sheets)
   {
      …
      Marshal.RealseComObject(sheet);
   }
   …
}

Catch()
{
}

Finally
{
   wb.Close(false, missing, missing);
   while (Marshal.ReleaseComObject(wb)!=0)
   {
   }
   wb=null;
   excel.Quit();
   while (Marshal.ReleaseComObject(excel)!=0)
   {
   }
   excel=null;
   GC.Collect();
   GC.WaitForPendingFinalizers();
}

Update:

I have following version of Finally

Finally{
wb.close(false, missing, missing);
excel.application.quit();
excel.quit();

Marshal.ReleaseComObject(wb);
Marshal.ReleaseComObject(excel);
GC.Collect();
GC.WaitForPendingFinalizers();
}

Which one is better (make more sense)? I have checked all the links here, but there is no finally answer, everybody's answer is sightly difference. I am not sure which one is the best?

GLP
  • 3,441
  • 20
  • 59
  • 91
  • Or [this](http://stackoverflow.com/questions/2926205/does-every-excel-interop-object-need-to-be-released-using-marshal-releasecomob) as well. – Brian Oct 31 '13 at 17:34

1 Answers1

0

The best practice is to call Marshal.ReleaseComObject(object name) or Marshal.FinalReleaseComObject(object name) on all of your COM objects. Of course, depending on the size of your project, this can become unwieldy very quickly. The code you have posted above should work just fine on a smaller scale and if you pay close attention to when/where your are creating the COM objects. I found some further reading for you here, here, here and here.

Community
  • 1
  • 1
Brian
  • 5,069
  • 7
  • 37
  • 47
  • Do I need to relase the excel.worksheet? Since I saw a few samples that don't release the worksheet in the foreach loop – GLP Nov 04 '13 at 21:40
  • @GLP - You _technically_ don't have to release them all, no. But, if you are working with a large number of them, you really _should_. They are considered 'unmanaged' as all things in the `interop` assemblies are. – Brian Nov 04 '13 at 21:47
  • So I think I should relase worksheet object. But in the foreach loop i couldn't set the sheet=null after the Marshal.RealseComObject(sheet). Does it mean I should not use foreach? Or I could creat a func like private void NAR(object o) { try { while (System.Runtime.InteropServices.Marshal.ReleaseComObject(o) > 0) ; } catch { } finally { o = null; } } Then I could indirectly set the sheet object to null. – GLP Nov 04 '13 at 22:04