Unit-testing the importing of data into a database

I have a functionality that imports data into a database, based on an Excel workbook and some meta data (both user-supplied). The functionality implements interface IFunctionality which essentially specifies an AuthId string property (used for fetching authorized AD groups for a functionality) as well as a CanExecute and Execute method.

The catch is that a Functionality must be COM-visible to be called via a legacy VB6 application and I want the COM interface as simple as it could be, and to me this means a IFunctionality implementation only has a parameterless constructor.

Below is the almost complete code (stripped a couple DAL calls and anonymized company-specifics) for one of those functionalities. I'm not going to supply DAL and Presentation layer code here, since my concerns are mainly around the Business Logic layer - but for the record I have DAL implemented with Linq to SQL and Presentation taken care of with WPF (which I just started learning).

```
[ComVisible(true)]
[ClassInterface(ClassInterfaceType.None)]
[ComDefaultInterface(typeof(IFunctionality))]
public class ImportFunctionality : IFunctionality
{
private ImportWindow _window; // a WPF view

public ImportFunctionality()
{ }

public bool CanExecute()
{
return CurrentUser.IsAuthorized(AuthId);
}

public string AuthId
{
get { return GetType().ToString(); }
}

///
/// Prompts for an Excel workbook filename and creates pricing tables from workbook data.
///
public void Execute()
{
try
{
if (!CanExecute()) throw new NotAuthorizedException(resx.Functionality_Execute_NotAuthorised);

var xlData = GetExcelSourceData();
if (xlData == null) return;

var viewModel = GetImportSettings(xlData);
if (viewModel == null) return;

if (!GetUserConfirmation(viewModel)) return;
ImportGridContent(viewModel);
}
catch (Not

The points you've specified in your question actually show that you (almost) see issues in your code yourselves, so thumbs up for that. I'll only expand on your points plus add a couple:

• your class is doing too much. You've noted that it has to be accessible from COM, but it doesn't mean that this exact class (business logic, BL) should be accessible from COM. What you should do is create a separate class/interface for accessing BL from COM (in terms of patterns it is called façade), and that class should wire up and issue calls to your BL, so that BL is not designed with restrictions/requirements imposed by COM;

• you should rewrite all your static classes/helpers like CurrentUser, FileDialogHelper and Excel8OleDbHelper to be instance classes implementing certain interfaces, and use them from main "functionality" class via interfaces only;

• do not use UI in business logic (MsgBox.Failure, _window.ShowDialog, MsgBox.Prompt, etc). The code that uses business logic should be responsible for providing data needed, and showing errors/messages to customer, and business logic should not reference any class from UI namespaces;

• you should not expose private methods as public/protected in order to unit-test them. Write as many unit tests (that use only public interface of your class) as you need to check all use cases of your class. Requirement "all use cases" ensures that they will verify all private methods as well;

• transactions should not depend on UI. If you need to expose progress of long-term operation - expose an event for that, but don't block on it. If you need a confirmation for final import - just expose another method that UI can call when user confirmed the import;

• extract DAL-related code into separate class with its own interface;

• wire up all the classes either manually in the COM-exposed class or using one of the IoC frameworks;

• test each class separately, mocking all external dependencies (that you should reference via interface) with one of the mocking frameworks (Moq, Rhino Mocks, etc)

• you haven't shown import code, but I would suggest looking into bulk upload methods, e.g. like suggested here.

StackExchange Code Review Q#24930, answer score: 6