tags:

views:

349

answers:

3

Hello all, I'm having a problem designing part of my program (not writing it, for once!). This is kind of hard to explain without writing a novel, so I'll try and be brief.

Basically, I have a program which reads/writes parameters from a piece of hardware. Currently, it does so over Serial, but eventually, I'll like it to do so over USB, using the .NET wrapper for the FTDI chip http://www.ftdichip.com/Projects/CodeExamples/CSharp.htm

I think my problem is, I know I want several layers of abstraction, but I can't seem to know where to draw the lines. First, I don't want my ReadParam(), WriteParam(), and SendCommand() functions to be sitting in my main form class. That just seems cobbled. So obviously they should be in some other class, which I'll instantiate. Let's call that Comm for now.

The first option is, I could make an interface, lets say IComm, and have my Serial and USB flavors both implement that. The problem with this is, a large percentage of the code would be duplicated in both flavors, because I have special ReadReplyData() and other functions, that do pre-processing of the serial data before they return it to the GUI.

So the next option, is have Comm be an intermediary class, which defines an interface ICommDriver. Comm would implement a private ReadReplyData() formatting function, as well as the public ReadParam(), WriteParam(), and SendCommand() functions, while ICommDriver would specify only simpler Read and Write functions.

This all seems trivial except for two twists. One, I want this to be multi-theaded, obviously, so the GUI doesn't hang. So I'm thinking that Comm would use a BackgroundWorker to do all the reads/writes. Also, the Serial flavor needs to be told which COM port to open (from a GUI drop-down), while the USB flavor does not. So do I make that part of the interface or not?

Thanks for your help everyone, I've been writing/deleting code for days trying to figure out the correct way to do this!

Jonathon

+1  A: 

In regards to exactly which kind of interfaces you need, that is ultimately up to you and whoever knows how this application works at that low of a level. I would like to respond to the part about using your implementations in a UI, and the comment about Comm using a BackgroundWorker. I would recommend inverting that. BackgroundWorker is really a UI level component....but Comm would be more of a "central" component (like a business object in an enterprise application). Your UI should create a BackgroundWorker that then creates Comm instances to perform the required work, and orchestrate any events from your Comm to update the UI. If you need your UI and the BackgroundWorker to communicate over a lengthy duration of time, I would recommend creating some kind of data transfer object that your UI can create, drop in a queue, and use ManualResetEvent or AutoResetEvent threading handles to communicate between your UI thread and the BackgroundWorker. That should give you a more loosely coupled product, allowing you to develop your Comm class independantly of any kind of UI that may display it (possibly allowing you to have WindForms, WPF, Command line, and maybe even PowerShell clients.)

jrista
+7  A: 

I know I want several layers of abstraction, but I can't seem to know where to draw the lines

This is where your problem lies. It is a fundamentally flawed approach to development and it is exactly what leads to this paralysis. Develop several concrete implementations of the flavors first. Get them working in your application with kludgy if type1 else type2 logic. Then go back and refactor them all to share a common contract, common base, what have you. It will be blindingly obvious what needs to go where.

With more details in the comments:

If you have shared code between implementations, you should use an abstract class. In my experience it's best practice to keep the public methods final and call protected abstract methods from the public methods, like so:

public interface IComm
{
    void WriteParam(...);
}

public abstract class CommStandardBase : IComm
{
    public void WriteParam(...)
    {
        DoWriteParam(...);
    }

    private void DoWriteParam(...)
    {
        CommonWrite1(...);
        HandleWriteParam(...);
        CommonWrite2(...);
    }

    protected abstract void HandleWriteParam(...);

    private void CommonWrite1(...)
    {
        ...
    }

    private void CommonWrite2(...)
    {
        ...
    }
}

Make each class self-contained. It should be single-instance, single-threaded and can be passed between workers and reporters.

Rex M
I already do have the software functioning, and I am trying to refactor and clean up the code. Like I mentioned, I HATE having my `ReadParam()` and `WriteParam()` functions in my MainForm class.The two flavors are basically just wrappers of the .NET SerialPort class, and FTDI's FTD2XX_NET class. So they are almost identical, except for the underlying functions they call.
Jonathon Reinhart
@Jonathon what's wrong with an abstract class which implements the shared functionality?
Rex M
I agree with that implementation when you have that shared code issue.
Saif Khan
Well don't mince words, Rex; tell him how you really feel. :)
Robert Harvey
@Robert :) http://headrush.typepad.com/creating_passionate_users/2005/08/the_smackdown_l.html
Rex M
No, I like it! I added that comment after letting the page sit for a while, and didn't see your edits. I think having an abstract class which implements the interface is just what I need. One public function, which calls the abstract protected function (along with other private functions). Makes sense now!
Jonathon Reinhart
I saw this discussed in another thread, but couldn't find their resolution. Does the abstract class need to implement all of the functions in the interface? I think this varies from language to language.
Jonathon Reinhart
@jonathan in c#, any class (abstract or not) which implements an interface must fulfill all the members. However, you don't have to provide implementations in an abstract class - e.g. if the interface has a void Foo(), an abstract void Foo() will satisfy the contract.
Rex M
+1  A: 

I am somewhat in VB.NET mode at the momement so here goes...


Interface IComm

    Function ReadParam()
    Function WriteParam()
    Function SendCommand()

End Interface



>


MustInherit Class CommBase

.... Load this up with the overideable

End Class



Then just implement the interface and inherit the base if needed. I also agree with Rex M. Don't push it too far for loose coupling.

Saif Khan