CODEWITHSUNDEEP
c#.netwinforms
b2f
b2f

Reputation: 3

Accessing GUI from another class via a static helper class

I just wanted to ask if the following code is a valid method to access the GUI from another class, or if it is bad practice. What I want to do is to write log messages into a RichTextBox in Form1.

If it's bad practice, would it be better to pass a reference of my Form1 to the other class to be able to access the RichTextBox.

I have the following code to access the GUI in my Form1 from another class:

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();             
        Logger.Init(this.rtbLog);

        MyOtherClass myOtherClass = new MyOtherClass();
        myOtherClass.DoSomething();
    } 
}

public class MyOtherClass
{
    public void DoSomething()
    {
        Logger.AppendText("text...");
        Logger.AppendText("text...");
        Logger.AppendText("text...");
    }
}

public static class Logger
{
    private static RichTextBox _rtb;

    public static void Init(RichTextBox rtb)
    {
        _rtb = rtb;
    }

    public static void AppendText(String text)
    {
        _rtb.AppendText(text);
        _rtb.AppendText(Environment.NewLine);
    }
}

With Events (thanks to Ondrej):

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();             

        Logger.EntryWritten += Logger_EntryWritten;

        MyOtherClass myOtherClass = new MyOtherClass();
        myOtherClass.DoSomething();
    }

    void Logger_EntryWritten(object sender, LogEntryEventArgs args)
    {           
        rtbLog.AppendText(args.Message);
        rtbLog.AppendText(Environment.NewLine);
    }
}

public class MyOtherClass
{
    public void DoSomething()
    {
        Logger.AppendText("text...");
        Logger.AppendText("text...");
        Logger.AppendText("text...");
    }
}

public static class Logger
{
    public static event EventHandler<LogEntryEventArgs> EntryWritten;

    public static void AppendText(string text)
    {
        var tmp = EntryWritten;
        if (tmp != null)
            tmp(null, new LogEntryEventArgs(text));
    }
}

public class LogEntryEventArgs : EventArgs
{
    private readonly String message;

    public LogEntryEventArgs(String pMessage)
    {
        message = pMessage;
    }

    public String Message
    {
        get { return message; }
    }
}

Upvotes: 0

Views: 469

Answers (1)

Ondrej Janacek
Ondrej Janacek

Reputation: 12616

It's probably fine for a small throw-away project, but otherwise a logger should not know anything about used platform. Then it would be good to use events for example. Raise an event whenever there's a new log entry written and consumers interested in logged entries will subscribe to a delegate.

Also be careful with threads. If you log a message from a different thread than UI you will end up with an exception because you would access a GUI control from a different thread which is forbidden.

EDIT:

Something along these lines. LogEntryEventArgs is a type you have to create and you can give it properties like Message, TimeWritten, Severity, etc.

public static class Logger
{
    public static event EventHandler<LogEntryEventArgs> EntryWritten;

    public static void AppendText(string text)
    {
        var tmp = EntryWritten;
        if (tmp != null)
            tmp(null, new LogEntryEventArgs(text));
    }
}

consumer:

Logger.EntryWritten += Logger_OnEntryWritten;

void Logger_OnEntryWritten(object sender, LogEntryEventArgs args)
{
    _rtb.AppendText(args.Message);
    _rtb.AppendText(Environment.NewLine);
}

Also, don't forget to invoke on a form/dispatch the body of Logger_OnEntryWritten in order to avoid cross-thread access exception (in case you are considering using threads).

Upvotes: 4

Related Questions