Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

In my application I have several event handlers that perform some action in response to user interface events such as a button click or menu selection. The code in these event handlers looks like this for example:

void MyDialog::OnCommandLoadFile()
{
    char strFilter[] = { "Text Files (*.txt)|*.txt|All Files (*.*)|*.*||" };

    CFileDialog fileDlg(TRUE, ".txt", NULL, 0, strFilter);

    if( fileDlg.DoModal() == IDOK )
    {
        const std::string path = fileDlg.GetPathName();

        std::vector<std::string> urls;
        int maxNumberOfUrls = settings_->Get<int>(Settings::MaxNumberOfUrls);
        UrlProvider *urlProvider = new FileUrlProvider(path);
        urlProvider->GetUrls(urls, maxNumberOfUrls);

        webMonitoringService_->MonitorUrls(urls);

        DisplayMessage("URLs loaded.");
    }

}

This is typical of my event handlers. I don't code monolithic event handlers but they do "glue" various pieces together. I usually have some business-logic/service classes which the UI has a reference to and in the event handlers I make calls to one or more of these to perform the desired task.

What I'm wondering is is this still too coupled? If I wanted to turn this into a command line app I'd have to extract bits and pieces from the UI code. In the above example, there is a compound action going on so should I'm thinking it should look more like:

void MyDialog::OnCommandLoadFile()
{
    char strFilter[] = { "Text Files (*.txt)|*.txt|All Files (*.*)|*.*||" };

    CFileDialog fileDlg(TRUE, ".txt", NULL, 0, strFilter);

    if( fileDlg.DoModal() == IDOK )
    {
        const std::string path = fileDlg.GetPathName();

        application->MonitorUrlsFromFile(path);

        DisplayMessage("URLs loaded.");
    }

}

Is that right? In the above code what would "application" be? A controller? Is that the right abstraction for this? Whenever I need to perform some task in a UI handler, should it always be a one-liner (aside from getting data in or out of text boxes etc.) Anyone have a pointer to a tutorial that really covers this (in any programming language)?

Furthermore, if in fact you should abstract compound actions, how to you propagate error messages from within the compound abstraction back out to the UI? An exception hierarchy? Single exception with error code?

UPDATE

I'm looking at the Command Pattern which looks promising.

UPDATE 2

In the end I decided on refactoring my code as described in the Humble Dialog Box paper in the accepted answer. My understanding is that this is a Model View Presenter (MVP) type pattern and more specifically a Passive View. The refactoring is going smoothly. There are some rough edges in my understanding of the full application of the pattern when it comes to things like displaying message boxes and file open dialogs or dealing with event timers but overall I'm happier with my code; the tight coupling was bothering me.

I also found this "bundle": patterns & practices Web Client Developer Guidance: Model View Presenter (MVP) Bundle which has sample MVP code. It's a web project whereas my current implementation is desktop but I guess that's the beauty of MVP is that it partly doesn't matter. One thing I got from that was to dependency inject my service layer objects into the presenter rather than new'ing them inside my presenter class.

share|improve this question
    
The best part of using the command pattern is how easy it becomes to implement unlimited undo/redo. –  Joeri Sebrechts Oct 17 '11 at 7:34

2 Answers 2

up vote 1 down vote accepted

If you want to learn how to better decouple your GUI from your business logic, I suggest you read Michael Feathers` article "The humble dialog box"

If you got yourself familiar with that, and you want some more of that stuff, here is a Blog series "Build Your Own CAB" which explains all important variants of "MVC":

http://codebetter.com/jeremymiller/2007/07/26/the-build-your-own-cab-series-table-of-contents/

EDIT: according your example code: when you are going to use "MVP", that will mean not to have a "god object" application where you place MonitorUrlsFromFile, but a controller (or "presenter") class MyDialogController at first hand. Think of it when your program get 20 dialogs and all of them would place their handler logic in a class application - that will easily end up with a class having 5000 lines of code or more, which is definitely too big.

By MVP approach, you will only refactor MonitorUrlsFromFile to another, more general place, when you need this function from more than one dialog and want to reuse it. Even then I would not make this an application class, better group functions for urls or url monitoring into one utility class.

share|improve this answer
    
I've read the humble dialog box. Article series looks interesting will check it out thanks. –  User Oct 16 '11 at 23:46
    
@User: see my edit. –  Doc Brown Oct 17 '11 at 5:48
    
Reread the humble dialog box and I got more out of if the second reading. Combining that with some other MVP info seems to be what I want. –  User Oct 17 '11 at 21:27

Your refactoring is fine, especially if MonitorUrlsFromFile() is duplicated code. You don't need a "decoupling from the UI" sensibility to justify that refactoring; it can be justified on its own merits.

The error propagation scheme depends on a number of factors. Generally, I use a BackgroundWorker object or thread or something similar to receive status events from a long-running background task, but the project I am currently working on has several business rule validations which will appear on the UI, so my approach is going to change there.

The simplest way to handle basic error propagation is to provide a top-level exception handler in your Main() method or whatever calls your processing loop.

share|improve this answer
    
do you think it makes sense to have a single "application" or "appController" object that holds all these compound actions (such as MonitorUrlsFromFile())? –  User Oct 14 '11 at 20:33
    
Kinda depends. If it's only used in the UI, it can stay in the UI somewhere. If it's just a utility method (one of many), it could go in a static class, along with the other methods. Don't call it application or appController, though. Either call it Utility or cluster common methods together and give it a meaningful name such as Url. Note that I am not an expert C++ programmer, and the C++ pedants might have some strong opinions about this. –  Robert Harvey Oct 14 '11 at 20:37

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.