[darkjohn20]

Alright, so my goal is to have a Form (with a ListView) that can be updated from one of my threads, but there are a few problems I have run into.

First of all, I'd like to have an array, or a list (not sure of the difference), to store the ListViewItems.
This way, instead of making a new item called 'item', I could make a new item stored in Array[5] so it can easily be deleted later using Array[5].Remove(), or something similar.

public delegate void AddListViewItemDelegate(ListViewItem lv);
public void AddListViewItem(ListViewItem lv)
{
	if (hClientList.InvokeRequired)
		Invoke(new AddListViewItemDelegate(AddListViewItem), new object[] { lv });
	else
		hClientList.Items.Add(lv);
}

private void hServerMainWindow_Load(object sender, EventArgs e)
{
	Thread NetworkThread = new Thread(new ThreadStart(NetThread));
	NetworkThread.IsBackground = true;
	NetworkThread.Start();
}

public void NetThread()
{
        //Code run when I want to create a new item (iClient contains index in array I want to store position)
        ListViewItem item = new ListViewItem("Client " + iClient); //Instead of 'item' I want it to be stored to an array at index iClient
        AddListViewItem(item);

        //code run when I want to delete an item
        //Remove an item using an integer corresponding to an index of the array, such as iClient again
}

Pseudo-code:
Create ListViewItemArray[100]
Create ListViewItem in array at [x], AND display it in my ListView
Delete ListViewItem from array [x], AND remove it from the ListView

Hopefully I am being clear enough.

[CodingSup3rnatur@l-360]

Well, first things first, the primary difference between an array and a list is that a list dynamically resizes itself. An array does not. Once you set the size of an array, that is the size you are stuck with unless you explicitly resize the array. .

Arrays implement IList and Lists implement IList<T>, and so both can be accessed using index notation i.e. ListViewItemArray[100].

This post has been edited by CodingSup3rnatur@l-360: 31 January 2013 - 03:59 PM

[darkjohn20]

As for Array vs List, I don't need it to be dynamic, so an Array will suffice.

This post has been edited by darkjohn20: 12 January 2011 - 03:29 PM

[Sergio Tapia]

Why don't you use the ObservableCollection<T> object and in the Collectionchanged event, repaint the collection?

[Curtis Rutland]

Well, first of all, your ListView already has a List of ListViewItems. It's a property called "Items". And from that, you can .Add and .Remove ListViewItems.

That's the difference between a List and an Array, by the way. Arrays are fixed upon creation. Lists are resizable, and have methods to "Add" and "Remove" items.

Now, on to thread saftey: remember that Action is a delegate too. So, you can simplify a bit. Actually, this example I'm about to paste is grossly oversimplified, but it should help:

void Form1_Load(object sender, EventArgs e)
{
    Task.Factory.StartNew(() =>
    {
        Thread.Sleep(5000);
        Invoke(new Action(() => label1.Text = "Task Finished"));
    });
}

The task is executed on a different thread, so it uses Invoke.

[darkjohn20]

My Delegate to update the list (code posted above) works perfectly. Instead of the variable item I'd like to use an array.

This is what I've added:

public static ListViewItem[] aItems = new ListViewItem[iMaxClients];

//Creation
aItems[iIndex].Text = "Item";
aItems[iIndex].SubItems.Add("SubItem");
AddListViewItem(aItems[iIndex]);

//Removal
aItems[iIndex].Remove();

However, I get the error: "Object reference not set to an instance of an object".

This post has been edited by darkjohn20: 12 January 2011 - 03:29 PM

[Sergio Tapia]

Can you elaborate, I've read post #6 twice and still can't figure out what you're asking there.

[CodingSup3rnatur@l-360]

That's because you haven't put anything in the array yet. You create an array which will have all it's elements set to null by default. You then try and access the element at index 'iIndex', but of course, there is no element at that index as you haven't filled the array yet.

This post has been edited by CodingSup3rnatur@l-360: 12 January 2011 - 03:31 PM

[tlhIn`toq]

Your sample code looks a lot like your other threads are directly affecting the ListView of the form.
That's a bad coding habit to get in to. Classes outside of the form shouldn't know anything about what's *in* the form. It's the form's job to handle the form. Its the other classes job to do their job. Keep the two separated. Let your other classes/threads just raise an event (with arguments if needed), and have your form subscribe to and react to those events.

Even within the same form you have multiple methods handling the listview. For example:

public void NetThread()
{
        //Code run when I want to create a new item (iClient contains index in array I want to store position)
        ListViewItem item = new ListViewItem("Client " + iClient); //Instead of 'item' I want it to be stored to an array at index iClient
        AddListViewItem(item);

        //code run when I want to delete an item
        //Remove an item using an integer corresponding to an index of the array, such as iClient again
}

I presume the job of the NetThread is to do something with the network. So why is is directly affecting the ListView? That's not it's job. Intead raise an event within the NetThread and have an event handler for that.

public void NetThread()
{
        //Code run when I want to create a new item (iClient contains index in array I want to store position)
        RaiseNewClientEvent(iClient);
}

void NewClientEventHandler(iClientArgs newClient)
{
           ListViewItem item = new ListViewItem("Client " + newClient);
           AddListViewItem(item);
}

In this way you can have a dozen different ways in which a new client might attach and they all simple raise an event notifying whomever is listening. One handler can then do it's one and only job of adding it to a list.

I think it's really important to keep methods doing one thing, but do it well, and not duplicate code all over. It just isn't a natural assumption that the ListView is affected by the "NetThread", "ReceiveThread", "NewClientThread", "DroppedClientThread", "NewHTTPthread" and so on.

This post has been edited by tlhIn'toq: 12 January 2011 - 03:33 PM

[darkjohn20]

Oh, alright. I've changed it to:

aItems[iIndex] = new ListViewItem(iIndex.ToString()); //Create ListViewItem with text representing its own index

It works, however, removing an item gives me a cross-thread error. Do I need to create another Delegate for:

aItems[iIndex].Remove();

Edit: Didn't see tlhIn'toq's post. Will fix that as well. Thanks for the tip!

This post has been edited by darkjohn20: 12 January 2011 - 03:35 PM

[darkjohn20]

Here's what seems to work. Any additional goods/bads would be appreciated.

public delegate void AddListViewItemDelegate(ListViewItem lv);
public void AddListViewItem(ListViewItem lv)
{
	if (hClientList.InvokeRequired)
		Invoke(new AddListViewItemDelegate(AddListViewItem), new object[] { lv });
	else
		hClientList.Items.Add(lv);
}
public delegate void RemoveListViewItemDelegate(ListViewItem lv);
public void RemoveListViewItem(ListViewItem lv)
{
	if (hClientList.InvokeRequired)
		Invoke(new RemoveListViewItemDelegate(RemoveListViewItem), new object[] { lv });
	else
		hClientList.Items.Remove(lv);
}

//Create
AddItem(iIndex, iIndex.ToString());

//Delete
RemoveItem(iIndex);

void AddItem(uint Index, string Text)
{
	aItems[Index] = new ListViewItem(Text);
	aItems[Index].SubItems.Add("SubItem");
	AddListViewItem(aItems[Index]);
}

void RemoveItem(uint Index)
{
	RemoveListViewItem(aItems[Index]);
}

This post has been edited by darkjohn20: 12 January 2011 - 03:48 PM

[CodingSup3rnatur@l-360]

Bear in mind that you can do something like this to add to the list view on the main thread from another thread all in one go:

this.Invoke(new Action<ListViewItem>((ListViewItem l) => this.hClientList.Items.Add(l)) , item);

That saves you quite a few lines of code as there is no need for you AddListViewItem method anymore.

It may be that you want to keep your method defined in full so you can call it from other parts of your code as it seems like quite a generalised method. You can still acomplish it in a simpler way than what you have:

public void AddListViewItem(ListViewItem lv)//keep your method so it can be called again from elsewhere if you need it again
{ 
hClientList.Items.Add(lv);
}

public void NetThread()
{
    ListViewItem item = new ListViewItem("Client " + "1");               
    this.Invoke(new Action<ListViewItem>(AddListViewItem), item); //marshall call over onto UI thread to update list view
}

It sort of depends on how often you are calling the Add and Remove methods as to how exactly you do it.

This post has been edited by CodingSup3rnatur@l-360: 24 December 2012 - 10:27 AM

[darkjohn20]

Simplified it a little bit:

public delegate void AddListViewItemDelegate(ListViewItem hLVItem);
public void AddListViewItem(ListViewItem hLVItem)
{
        hClientList.Items.Add(hLVItem);
}

//Create (I want to keep this function)
void AddItem(uint iIndex, string sText)
{
	aItems[iIndex] = new ListViewItem(sText);
	aItems[iIndex].SubItems.Add("Text");
	Invoke(new Action<ListViewItem>(AddListViewItem), aItems[iIndex]); 
}

//Remove (Replaced my remove function)
Invoke(new Action<ListViewItem>((ListViewItem lv) => hClientList.Items.Remove(lv)), aItems[iIndex]);

Would this be better than what tlhIn'toq suggested (Thread that calls a Function which calls the Delegate)?

This post has been edited by darkjohn20: 12 January 2011 - 04:30 PM

[CodingSup3rnatur@l-360]

No. It is a VERY good idea to follow what tlhIn'toq said. What I put is just to show you that it is usually much simpler to make cross thread calls than your code was suggesting, and it was in the context of your application as it stands currently.

If we are talking about the design of your application as a whole, without question, you should be doing what tlhIn'toq says.

In my mind, the form class should handle GUI only, and all classes should be responsible for ONE thing. All methods within those classes should do ONE thing each.

Always look to use events. The network thread wouldn't be explicitly calling a method in the form class, that method would just get called when a client is found in the network thread. Nothing regarding the specifics of the networking side of your application should be in a form class. The form class handles the GUI. Period. Any networking logic should be split out into a separate class.

This post has been edited by CodingSup3rnatur@l-360: 12 January 2011 - 04:55 PM

[AdamSpeight2008]

Thread Safe and Cross Threading are two different things.