0

So, For the sake of learning how to use threading I made a little program that is supposed to take in names and store them in a list and then show the names in another form one after another. So, here is the code:

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows.Forms;
using System.Threading;

namespace ListingThread
{
public partial class Form1 : Form
{
    public Thread t;
    public static int ShowNo = 0;
    public Form1()
    {
        InitializeComponent();
        GlobalForm = new Form();
        GlobalForm.Show();
        GlobalForm.Hide();
        t = new Thread(ShowList);
    }
    public Form GlobalForm;
    public static List<string> Names = new List<string>();
    private void Form1_Load(object sender, EventArgs e)
    {

    }

    private void btnAddName_Click(object sender, EventArgs e)
    {
        AddName(txtName.Text);
        txtName.Text = null;
    }
    public void AddName(string x)
    {
        Names.Add(x);
    }

    private void btnShow_Click(object sender, EventArgs e)
    {
        Showw();
    }
    public void Showw()
    {
        t.Start();
    }
    public void ShowList()
    {
        if (ShowNo < Names.Capacity)
        {

            GlobalForm.Invoke((MethodInvoker)delegate ()
            {
                CustomMessageBox Msg = new CustomMessageBox();
                try { Msg.lblName.Text = Names[ShowNo];
                }
                catch
                {
                    t.Abort();
                }
                Msg.Show();
                Thread.Sleep(500);
                Msg.Close();
                ShowNo++;
                ShowList();
            });
        }
        else
        {
            t.Abort();
        }
    }
}
}

The Other form simply has one label named lblName: The Second Form

But When I run this happens :/ : Program Running

As you can see, the labels don't work properly :/ They are shown like this: Broken Label

Glorfindel
  • 21,988
  • 13
  • 81
  • 109
Anopey
  • 351
  • 2
  • 3
  • 10
  • `ShowNo < Names.Capacity` -> `Names.Count` perhaps? – AlexD Jan 23 '17 at 20:05
  • I changed it to what you said but it still doesn't work :/. I think the problem here is a bit more complicated. – Anopey Jan 23 '17 at 20:15
  • I blieve forms have to run on the UI thread. – stuartd Jan 23 '17 at 20:19
  • 1
    Pretty simple really. You are sleeping, which blocks the label from getting refreshed. Use a timer instead. You can cheat by calling `Msg.Refresh();` before the sleep call. This isn't the best way to learn threads, by the way. Threads are used to manipulate long running operations, such as data retrieval, analysis, etc. Any GUI stuff has to happen on the main thread. Your GlobalForm isn't very useful. – LarsTech Jan 23 '17 at 20:35
  • You are sleeping the UI thread, which is blocking the call to `Invoke`. Have you tried using `BeginInvoke` instead of `Invoke`? That shouldn't block, it just queues the invocation. – Bradley Uffner Jan 23 '17 at 20:36
  • Also, never *ever* **ever** do this `t.Abort();` That will put your application in a completely unpredictable state. https://stackoverflow.com/questions/1559255/whats-wrong-with-using-thread-abort – Bradley Uffner Jan 23 '17 at 20:42

1 Answers1

1

I would suggest using a Timer instead of Thread.Sleep so as to not block the UI thread, as sleeping will prevent the label from updating. 

while (ShowNo < Names.Count)
        {
            CustomMessageBox Msg = new CustomMessageBox();
            GlobalForm.Invoke((MethodInvoker)delegate()
            {
                try
                {
                    Msg.lblName.Text = Names[ShowNo];
                    Msg.Show();
                    System.Windows.Forms.Timer timer = new System.Windows.Forms.Timer();
                    timer.Interval = 2000;
                    timer.Tick += (s, e) => Msg.Close();
                    timer.Start();
                }
                catch
                {
                    t.Abort();
                }
                ShowNo++;
            });
        }

As an aside, there's no need to abort the thread once it's finished running. I'd also use a while loop instead of a recursive method call.

Clivey
  • 88
  • 1
  • 6