C#, separating message display code from the business logic

I have a winforms application and I'm not following any kind of design pattern. My problem is, I have these base classes which contain all my business logic. When ever an exception occurs or I need to display a dialog box to the user, I have written the code directly into the base classes where I need it.

I know that I need to separate my business logic and display logic, so I wrote a static class that includes the methods that I need to display the messages with.

My question is, is there an easier way to separate business logic from the display?

my static methods look like this,

public static void DisplayMessage(string message) 
    {
        MessageBox.Show(message);
    }

 public static bool DisplayDialogBox(string message,string caption ) 
    {
        DialogResult newresult = new DialogResult();

        newresult = MessageBox.Show(message,caption,MessageBoxButtons.OKCancel);

        if (newresult == DialogResult.OK)
        {
            return true;
        }
        else 
        {
            return false;
        }  

So I'll be calling these methods from the base classes, like

MsgDisplay.DisplayMessage(e.Message);

a开发者_如何学Cnd is this method a good practice?

No, this method is not a good practice. There is no any difference between your class and MessageBox class.
See, with your class:

MsgDisplay.DisplayMessage(e.Message);

and just using MessageBox

MessageBox.Show(e.Message);

This wrapper doesn't provide any additional functionality.
If you want to separate business logic and ui, then you have to break down your methods and display message only in UI layer.
And, small point. Instead of:

if (newresult == DialogResult.OK)
    {
        return true;
    }
    else 
    {
        return false;
    }

type just:

return newresult==DialogResult.OK

UPDATE: If all you want to display is exception message then you should catch exception and show message on UI layer. So in your business class instead of displaying message:

void foo() {
   try {
       //some code here
   }
   catch(FooException fe) {
       MessageBox.Show(fe.Message);
   }
}

throw exception to the ui layer:

void foo() {
   try {
       //...
   }
   catch(FooException fe) {
       //throw new BarException("Error occured: "+fe.Message); //if you want to customize error message.
       throw; //If you don't need to change the message consider to not catch exception at all
   }
}

and then display the message outside of the business logic:

void fooButton_Click(object sender, EventArgs args) {
   try {
        fooBusinessClass.foo();
   } catch(FooException fe) {
        //if(MessageBox.Show(fe.Message)==DialogResult.OK) fooBusinessClass.continuefoo(); //if you have several options
        MessageBox.Show(fe.Message);
   }
}

I typically create an IView interface which is then injected into the business logic classes. If the logic "has a question" it needs to get user input it would then go like this:

interface IView
{
     bool AskUser(string question);
}

class SomeClass
{
     IView _View;

     public SomeClass(IView view)
     {
        _View = view;
     }

     public void SomeLogic()
     {
          if (someCondition && !_View.AskUser("continue?"))
               return;
     }
}

And then your form can implement IView to ask the user via message box prompt. For unit testing you can mock out the view which you can't really do in the static design case.

It's easier than you might think implementing a simple MVC-ish design pattern in WinForms, and you can bolt this onto existing code without significant modifications. Have your form or control implement a view interface, and pass the view to the class that implements your business logic:

public interface IPersonDetailsView
{
    bool PromptUser(string message, string caption);
}
// Your form:
public partial class PersonDetailsForm : Form, IPersonDetailsView
{
    //...
    public bool PromptUser(string message, string caption) {
        var result = MessageBox.Show(message, caption, MessageBoxButtons.OkCancel);
        return result == DialogResult.Ok;
    }
}
// Your business logic:
public class PersonDetailsController {
    public IPersonDetailsView View { get; set; }

    public void DoingSomething() {
        // ...
        if (this.View.PromptUser(message, caption)) { ...
        }
    }
}

Set the PersonDetailsController.View to the form when it is created. If you need the form to be able to talk back to the controller you can just add a PersonDetailsForm.Controller and let the form call public methods on the controller.

Rather than just using the form as a proxy for the WinForms calls I would use a BDD approach, so rather than View.ShowPrompt("Do you want to delete this person?", "Deleting person") I would go for something like View.AskUserIfTheyWantToDeleteThePerson() (with no arguments). That's a long method name but it is very explicit, leaves the implementation and message up to the view and can make things clearer in the long run.

Generally, the business layer returns an error string. The string is displayed in a MessageBox or status bar by the GUI.

By the way, you can get a result from a message box (you don't need a dialog):

MessageBoxResult MBR = MessageBox.Show("Click Me", "Title", MessageBoxButton.YesNoCancel);

MessageBox.Show("You selected: " + MBR.ToString());

I would take an approach of using events to surface this kind of message display. You can then easily decide if you want to log or not by subscribing to the events.

Here's how I would do it:

First define a couple of delegates for your message methods:

public delegate void DisplayMessage(string message);
public delegate bool DisplayDialogBox(string message, string caption);

These can be used as events in your business logic classes:

public class BusinessLogic
{
    public event DisplayMessage DisplayMessage;
    public event DisplayDialogBox DisplayDialogBox;

    protected void OnDisplayMessage(string message)
    {
        var dm = this.DisplayMessage;
        if (dm != null)
        {
            dm(message);
        }
    }

    protected bool OnDisplayDialogBox(string message, string caption)
    {
        var ddb = this.DisplayDialogBox;
        if (ddb != null)
        {
            return ddb(message, caption);
        }
        return false;
    }

    public void SomeMethod()
    {
        this.OnDisplayMessage("Hello, World!");
        var result = this.OnDisplayDialogBox("Yes or No?", "Choose");
        this.OnDisplayMessage(result.ToString());
    }
}

Now the calling code looks like this:

var bl = new BusinessLogic();

bl.DisplayMessage += MsgDisplay.DisplayMessage;
bl.DisplayDialogBox += MsgDisplay.DisplayDialogBox;

bl.SomeMethod();

And this works nicely in my tests.

Now, one warning - the DisplayDialogBox delegate returns a bool so when it is used as an event handler you could have multiple subscribers attach to the event and then only the last return value is returned, but all of the subscribers will handle the event. You could have the dialog box pop-up, the user says "No", but the next handler returns "Yes" and that's what's returned back.

There is a relatively easy fix. Replace the line return ddb(message, caption); with:

            return ddb
                .GetInvocationList()
                .Cast<DisplayDialogBox>()
                .Select(d => d(message, caption))
                .Aggregate((b1, b2) => b1 || b2);

As long as you choose an appropriate aggregation function - ||, && - or group by the bool and choose the one with the highest count - then it'll work great.

Let me know if this helps.