How to keep my functions (objects/methods) 'lean and mean'

Question

Hi,

in all (Agile) articles i read about this: keep your code and functions small and easy to test.

How should i do this with the 'controller' or 'coordinator' class?

In my situation, i have to import data. In the end i have one object who coordinates this, and i was wondering if there is a way of keeping the coordinator lean(er) and mean(er).

My coordinator now does the followling (pseudocode)

//Write to the log that the import has started
Log.StartImport()
//Get the data in Excel sheet format
result = new Downloader().GetExcelFile()
//Log this step
Log.LogStep(result )
//convert the data to intern objects
result = new Converter().Convertdata(result);
//Log this step
Log.LogStep(result )
//write the data
result = Repository.SaveData(result);
//Log this step
Log.LogStep(result )

Imho, this is one of those 'know all' classes or at least one being 'not lean and mean'? Or, am i taking this lean and mean thing to far and is it impossible to program an import without some kind of 'fat' importer/coordinator?

Michel

EDIT this is actually a two-in-one question: one is how to test it, second is if it's ok to have a 'know all/glue all together' coordinator

Answer 1

I had a similar problem and the way I solved it was to really have a proper look at the SRP (Single Responsibility Principal).

ExcelFileReader who can read the excel file and return a set of List (lines)

Parser it's job is to parse the lines returned from the ExcelFileReader using the delimiter

Importer which handles importing the DataSet returned from FileParser into the correct tables.

This keeps it in a testable form as ExcelFileReader doesn't depend on the Parser or Importer. This goes for the Parser as well as it's testable by just passing it a TextReader.

Does this help?

Answer 2

It is very common to have a class that does what I believe is the mediator pattern.

One way I have found to combat classes that have to have collaborators (which is what you are running into right now) is to do Mock Unit Testing.

You essentially Mock your collaborators and set expectations to happen to those collaborators.

Unfortunately you are using C# and I do not know of any Mock libraries for C#. However I am sure google can help you on find a mock library.

Instead of a mock library you can just implement or extend your collaborator's classes and override and implement the methods that you expect to call with methods that produce expected output.

As noted by Michael it makes it far easier if you have dependency injection wiring in your collaborators.

Answer 3

I am sure that many people would disagree, but I think your method is in general lean enough. The crucial bit you are missing here though is dependency injection (also known as inversion of control) - so instead of newing Downloader, Converter etc. inside your method, you should define interfaces for these classes and "inject" them in the constructor of your class:

private Downloader downloader;
private Converter converter;

public MyClass(Downloader downloader, Converter converter)
{
  this.downloader = downloader;
  this.converter = converter;
  //same with repository, it can't be a static class anymore
}

then you just use these instances in your method:

//Write to the log that the import has started
Log.StartImport()
//Get the data in Excel sheet format
result = downloader.GetExcelFile()
//Log this step
Log.LogStep(result )
//convert the data to intern objects
result = converter.Convertdata(result);
//Log this step
Log.LogStep(result )
//write the data
result = repository.SaveData(result);
//Log this step
Log.LogStep(result )

now after doing this change, in your test you can use one of the mocking frameworks (I use RhinoMocks) to inject a mocked implementation of the dependencies into your class. This way you can just verify that the correct method was called on the converter and downloader, without reading anything from disk and parsing any spreadsheets.

Examples of how to use RhinoMocks are in their documentation: http://ayende.com/wiki/Rhino+Mocks+Documentation.ashx

Don't hesitate to ask another question if you get stuck :)

Answer 4

I found this article particularly useful when deciding if my code was indeed lean and mean

Answer 5

Your comments and logger (singleton?) are too noisy:

Log.StartImport();
result = new Downloader().GetExcelFile()
Log.Step(result);
result = new Converter().Convertdata(result);
Log.Step(result);
result = Repository.SaveData(result);
Log.Step(result);

And since you have three steps that have to happen together wrap, them up in Converter. Using DI pass in to Converter a Downloader and a Repository. Now you end up with:

Log.StartConvert();
convert.ExcelToRepository(); //passed in with DI

Note all Log step results are done in there respective actions.