0

In one of my window form, I created an instance of a class to do some works in the background. I wanted to capture the debug messages in that class and displayed in the textbox in the window form. Here is what I did:

class A //window form class
{
   public void startBackGroundTask()
   {
      B  backGroundTask = new B(this);
   }

   public void updateTextBox(string data)
   {
        if (data != null)
        {
            if (this.Textbox.InvokeRequired)
            {
                appendUIDelegate updateDelegate = new appendUIDelegate(updateUI);

                try
                {
                    this.Invoke(updateDelegate, data);
                }
                catch (Exception e)
                {
                    Console.WriteLine(e.Message);
                }
            }
            else
            {
                updateUI(data);
            }
        } 
   }

    private void updateUI(string data)
    {
        if (this.Textbox.InvokeRequired)
        {
            this.Textbox.Invoke(new appendUIDelegate(this.updateUI), data);
        }
        else
        {
            //update the text box
            this.Textbox.AppendText(data);
            this.Textbox.AppendText(Environment.NewLine);
        }
    }

    private delegate void appendUIDelegate(string data);
}

class B // background task
{
   A curUI;

   public b( A UI)
   {
      curUI = UI;
   }

   private void test()
   {
      //do some works here then log the debug message to UI.
      curUI.updateTextBox("message);
   }
}

I keep getting a null reference exception after

this.Invoke(updateDelegate, data);

is called.

I know passing "this" as a parameter is strange, but I want to send the debug message to my window form.

Rob
  • 26,989
  • 16
  • 82
  • 98
alex
  • 275
  • 1
  • 7
  • 16
  • For anyone having a similar issue, I found that my background service was trying to update the UI before the form had been fully initialised thus causing the null reference. – Daniel Mar 31 '20 at 09:19

3 Answers3

3

Two things:

1) Consider using an extension method so you don't repeat yourself on all those invokes:

public static class ControlExtentions
{
    public delegate void InvokeHandler();
    public static void SafeInvoke(this Control control, InvokeHandler handler)
    {
        if (control.InvokeRequired)
            control.Invoke(handler);
        else
            handler();
    }
}

2) Consider using events to make this more readable. Use something like this design:

class B
{
    public event EventHandler LogGenerated = delegate { };

    void test()
    {
        LogGenerated("Some log text", EventArgs.Empty);
    }
}

class A : Control
{
    public A()
    {
        B b = new B();
        b.LogGenerated += new EventHandler(b_LogGenerated);
    }

    void b_LogGenerated(object sender, EventArgs e)
    {
        this.SafeInvoke(() => { textBox1.Text += (String)sender; });
    }
}

Don't let that syntax of calling SafeInvoke throw you too much; it's simply a lambda that represents an InvokeHandler delegate.

drharris
  • 11,194
  • 5
  • 43
  • 56
  • That's all very nice but it doesn't actually answer the question. And please don't initialize event fields like that as a lazy way of avoiding the null check; it's not the same thing, it's *way* more expensive to invoke an empty multicast delegate than it is to check for a null reference. – Aaronaught May 18 '10 at 21:51
  • I initialized the event as an empty delegate for sake of brevity to ensure it works as-is. I assume (perhaps incorrectly) that most people know to check it properly. – drharris May 18 '10 at 21:55
2

The logic you've described in the question does not produce a null reference exception, which suggests that the exception is coming from the background task you're executing rather than from the UI update logic.

As evidence, here's a full sample that works which is simplified, but based on the pattern you described:

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

    private void updateTextBox(string data)
    {
        if (this.textBox1.InvokeRequired)
        {
            this.Invoke(new MethodInvoker(() => updateTextBox(data)));
            return;
        }

        if (data == null)
        {
            return;
        }

        //update the text box
        this.textBox1.AppendText(data);
        this.textBox1.AppendText(Environment.NewLine);
    }

    private void _uxStartBgTask_Click(object sender, EventArgs e)
    {
        new Thread(() => updateTextBox("message")).Start();
    }
}

Note that the use of MethodInvoker eliminates the need to declare a delegate member and that immediate recursion is a convenient way of addressing the InvokeRequired pattern.

hemp
  • 5,602
  • 29
  • 43
0

It looks like you have an infinite loop. updateTextBox constructs a delegate to call updateUI, and updateUI constructs a delegate to updateUI. What prevents infinite recursion here?

dthorpe
  • 35,318
  • 5
  • 75
  • 119
  • No infinite recursion here. InvokeRequired will only return true if the call is coming from a non-UI thread. That won't be the case when the delegate is called via Invoke. – hemp May 18 '10 at 21:43