patterncsharpMinor
Tuning Excel calculation engine which uses MS Excel interop
Viewed 0 times
engineinteropcalculationexceltuninguseswhich
Problem
I am currently building an Excel calculation engine. Its purpose is basically to wrap the calculation logic of an Excel workbook in order to use the logic from a C# library.
The library is meant to work for arbitrary workbooks and a flexible number of cell writes and reads. I cannot optimize anything specifically for a workbook or a known workbook layout. The worksheet might contain VBA macros and functions. I've tried a lot to avoid Interop, but have not yet managed to find any solution.
Here is my wrapper:
```
public class Spreadsheet : IDisposable
{
[DllImport("user32.dll")]
private static extern uint GetWindowThreadProcessId(IntPtr hWnd, out uint lpdwProcessId);
private readonly Dictionary _worksheets = new Dictionary();
private readonly Dictionary _cells = new Dictionary();
private Application _excelApplication;
private Workbooks _workbooks;
private Workbook _workbook;
private Windows _windows;
private Window _windowItem;
private bool _autoCalculate;
public bool AutoCalculate
{
get { return _autoCalculate; }
set
{
_autoCalculate = value;
if (_excelApplication != null)
_excelApplication.Calculation =
_autoCalculate ? XlCalculation.xlCalculationAutomatic : XlCalculation.xlCalculationManual;
}
}
public Spreadsheet()
{
OpenExcelApplication();
}
private void OpenExcelApplication()
{
_excelApplication = new Application {Visible = false};
using (var s = new Spreadsheet())
{
s.AutoCalculate = false;
s.LoadFromFile("D:/workbook.xlsx");
// Example: Set B36 to 1000.0
s.SetValue(1000.0/i, "SheetName", 36, 2);
// Arbitrary cell values could be set here
// Calculate result values
s.Calculate();
// Example: Read B52
var result = s.GetValue("SheetName", 52, 2));
// Arbitrary cell values could be read here
}The library is meant to work for arbitrary workbooks and a flexible number of cell writes and reads. I cannot optimize anything specifically for a workbook or a known workbook layout. The worksheet might contain VBA macros and functions. I've tried a lot to avoid Interop, but have not yet managed to find any solution.
Here is my wrapper:
```
public class Spreadsheet : IDisposable
{
[DllImport("user32.dll")]
private static extern uint GetWindowThreadProcessId(IntPtr hWnd, out uint lpdwProcessId);
private readonly Dictionary _worksheets = new Dictionary();
private readonly Dictionary _cells = new Dictionary();
private Application _excelApplication;
private Workbooks _workbooks;
private Workbook _workbook;
private Windows _windows;
private Window _windowItem;
private bool _autoCalculate;
public bool AutoCalculate
{
get { return _autoCalculate; }
set
{
_autoCalculate = value;
if (_excelApplication != null)
_excelApplication.Calculation =
_autoCalculate ? XlCalculation.xlCalculationAutomatic : XlCalculation.xlCalculationManual;
}
}
public Spreadsheet()
{
OpenExcelApplication();
}
private void OpenExcelApplication()
{
_excelApplication = new Application {Visible = false};
Solution
One thing that I saw that didn't look good was, your resource disposal here
I don't think that you should use a Try-Catch here.
If you hit an exception while trying to close the application it probably means that some other resource is probably in use, you probably want the user to know that something is still being used. letting the exception bubble up is probably one of the best things that you can do here, aside form a popup that gives you the stack trace.
you should probably make good use of the Disposable Pattern of the Interface as well.
looking at the MSDN for IDisposable
When they implement the disposable pattern they don't use a try-catch in Snippet # 3
if you don't make sure that you are disposing the application correctly you will end up with Memory leaks.
it looks like you have created 2 Methods that do relatively the same thing
one returns a boolean and the other does not. you should be using the one that does not return anything in your Dispose Method. the way you are calling the current method doesn't make use of the Boolean that is returned.
public void Dispose()
{
try
{
var hWnd = _excelApplication.Application.Hwnd;
TryKillProcessByMainWindowHwnd(hWnd);
}
catch (Exception)
{
}
}I don't think that you should use a Try-Catch here.
If you hit an exception while trying to close the application it probably means that some other resource is probably in use, you probably want the user to know that something is still being used. letting the exception bubble up is probably one of the best things that you can do here, aside form a popup that gives you the stack trace.
you should probably make good use of the Disposable Pattern of the Interface as well.
looking at the MSDN for IDisposable
When they implement the disposable pattern they don't use a try-catch in Snippet # 3
class BaseClass : IDisposable
{
// Flag: Has Dispose already been called?
bool disposed = false;
// Instantiate a SafeHandle instance.
SafeHandle handle = new SafeFileHandle(IntPtr.Zero, true);
// Public implementation of Dispose pattern callable by consumers.
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}
// Protected implementation of Dispose pattern.
protected virtual void Dispose(bool disposing)
{
if (disposed)
return;
if (disposing) {
handle.Dispose();
// Free any other managed objects here.
//
}
// Free any unmanaged objects here.
//
disposed = true;
}
}if you don't make sure that you are disposing the application correctly you will end up with Memory leaks.
it looks like you have created 2 Methods that do relatively the same thing
public static bool TryKillProcessByMainWindowHwnd(int hWnd)
{
uint processId;
GetWindowThreadProcessId((IntPtr) hWnd, out processId);
if (processId == 0) return false;
try
{
Process.GetProcessById((int) processId).Kill();
}
catch (ArgumentException)
{
return false;
}
catch (Win32Exception)
{
return false;
}
catch (NotSupportedException)
{
return false;
}
catch (InvalidOperationException)
{
return false;
}
return true;
}
public static void KillProcessByMainWindowHwnd(int hWnd)
{
uint processId;
GetWindowThreadProcessId((IntPtr) hWnd, out processId);
if (processId == 0)
throw new ArgumentException("Process has not been found by the given main window handle.", "hWnd");
Process.GetProcessById((int) processId).Kill();
}one returns a boolean and the other does not. you should be using the one that does not return anything in your Dispose Method. the way you are calling the current method doesn't make use of the Boolean that is returned.
Code Snippets
public void Dispose()
{
try
{
var hWnd = _excelApplication.Application.Hwnd;
TryKillProcessByMainWindowHwnd(hWnd);
}
catch (Exception)
{
}
}class BaseClass : IDisposable
{
// Flag: Has Dispose already been called?
bool disposed = false;
// Instantiate a SafeHandle instance.
SafeHandle handle = new SafeFileHandle(IntPtr.Zero, true);
// Public implementation of Dispose pattern callable by consumers.
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}
// Protected implementation of Dispose pattern.
protected virtual void Dispose(bool disposing)
{
if (disposed)
return;
if (disposing) {
handle.Dispose();
// Free any other managed objects here.
//
}
// Free any unmanaged objects here.
//
disposed = true;
}
}public static bool TryKillProcessByMainWindowHwnd(int hWnd)
{
uint processId;
GetWindowThreadProcessId((IntPtr) hWnd, out processId);
if (processId == 0) return false;
try
{
Process.GetProcessById((int) processId).Kill();
}
catch (ArgumentException)
{
return false;
}
catch (Win32Exception)
{
return false;
}
catch (NotSupportedException)
{
return false;
}
catch (InvalidOperationException)
{
return false;
}
return true;
}
public static void KillProcessByMainWindowHwnd(int hWnd)
{
uint processId;
GetWindowThreadProcessId((IntPtr) hWnd, out processId);
if (processId == 0)
throw new ArgumentException("Process has not been found by the given main window handle.", "hWnd");
Process.GetProcessById((int) processId).Kill();
}Context
StackExchange Code Review Q#69301, answer score: 6
Revisions (0)
No revisions yet.