views:

86

answers:

3

Hi guys,

Basically, there is a system at my work place that provides OCR capabilities. The process is that, a third party application is configured to display a captured screen (during the OCR process) and a user sits at the pc making sure the captured data is correct.

This capture stage has validation on each of the fields. for example if the document is an invoice for a specific client, the supplier of the invoice is verified against reference data. The verification code is in the form of a compiled .net dll that is generated by myself, from a visual studio 2008 solution.

The third party interfaces are used in communicating between the capture form and the code I write. An example is;

#region GetLinesTotal
/// <summary>
/// Gets the total for e.g. all VAT lines from the table
/// </summary>
/// <param name="oCSM">ITisClientServicesModule</param>
/// <param name="oTab">field table object</param>
/// <param name="fieldName">partial fieldname of table field (without the $XXXX)/param>
/// <returns>total as a string, empty string if all values empty</returns>
public static string GetLinesTotal(ITisClientServicesModule oCSM,ITisFieldTableData oTab, string fieldName )
{
    string sLineTot = string.Empty;
    ErrHandling.TryInit(oCSM);
    string sFunction = "GetLinesTotal";
    try
    {
        decimal dTot = 0m;              
        string sTemp = string.Empty;
        for (int i = 0; i< oTab.NumberOfRepetitions;i++)
        {
            sTemp = Utils.GetFieldCont(oTab.ParentForm,fieldName + "$" + i.ToString("X").PadLeft(4,'0')).Trim();
            if (sTemp != string.Empty)
            {
                dTot += Convert.ToDecimal(sTemp);
                sLineTot = dTot.ToString();
            }
        }

    }
    catch (Exception ex)
    {
        ErrHandling.errHandler.LogMsg(ex.ToString(),sFunction,CLASS_NAME,TIS_SEVERITY.TIS_ERROR);
        sLineTot = "INVALID";
    }
    return sLineTot;
}
#endregion GetLinesTotal

What I wish to do, is to create a layer of abstraction, removing the 3rd party interfaces from this code (separating the concerns), which will allow for easier testing (TDD) etc.

I am new to these approaches, and I apologise if I have made any wrong assumptions. I was just wondering if I could get some advice on how to go forward with the code. At some point we(the company) may chose to go with a different 3rd party OCR application.

Thanks in advance

+1  A: 

Write an interface that defines the operations you want to carry out - from a client (i.e. your code) point of view - on the third-party interface, and a thin wrapper around the third party interface that implements that interface. Then you can provide a mock object in place of the interface for unit testing purposes, and you have decoupled the actual third party implementation so that switching it out becomes a matter of writing an alternative thin wrapper implementing your interface.

David M
+1 While the other answers focus on YAGNI (always a good principle), enabling testability is also important.
Mark Seemann
Apologies, we definitely will be moving away from the current 3rd party OCR tools
warpcore
Could I get an example of this implementation from the source above?
warpcore
A: 

I wouldn't worry about this until you decide to go with another OCR system. You have already written the code to the interface, so you already have a degree of separation for your unit tests (you can create mocks or stubs against the 'ITisClientServicesModule' and 'ITisFieldTableData' interfaces).

If I was to do anything at the moment, it would be to derive new interfaces that fold in your utility methods (ErrHanding.Initialise() and Utils.GetFieldCount()), so that your code coupling is reduced further.

Dr Herbie
Even if the code is already written and works, it may by Michael Feathers' definition be **legacy code**. The OP may wish to apply unit tests to the existing code before refactoring, and introducing **Seams** may help him do so.
Mark Seemann
Wouldn't you consider coding to the interface to be a seam already?
Dr Herbie
The "utility methods" are also part of the third party libraries
warpcore
@Dr Herbie: Not if the interface is defined in the same package as the implementation. As it looks now, you can't get rid of the implementation because that would also remove the interface. A **Seam** is defined by the consumer, not the implementer.
Mark Seemann
Thanks, I see the distinction now.
Dr Herbie
+1  A: 

At some point we may...

YAGNI warning!

Focus on the stuff you know is needed today. For the sake of TDD, there is no rule that says that you must create layers of abstraction. In fact, introducing unneeded abstractions may even be detrimental to the code (code bloat). If the day comes when you need to support more than one OCR implementation, only then do you need to create that abstraction.

Note that you can always mock the 3rd party code if it is hard to test and/or relies on stuff like databases, network och file access which is not really suitable for unit testing.

Martin Wickman
This is only true if the current code is testable at all... **Seams** enable testability, and introducing a seam may be what is needed to move forward.
Mark Seemann
Apologies, we definitely will be moving away.
warpcore