[KFredje]

Hi, I first show you my code:

public void ListView()
        {
            ContextMenuStrip mnu = new ContextMenuStrip();
            ToolStripMenuItem mnuAdd = new ToolStripMenuItem("Add");
            ToolStripMenuItem mnuEdit = new ToolStripMenuItem("Edit");
            ToolStripMenuItem mnuInfo = new ToolStripMenuItem("Info");
            mnuAdd.Click += new EventHandler(mnuAdd_Click);
            mnuEdit.Click += new EventHandler(mnuEdit_Click);
            mnuInfo.Click += new EventHandler(mnuInfo_Click);
            mnu.Items.AddRange(new ToolStripItem[] { mnuAdd, mnuEdit, mnuInfo });

            imageListLarge.Images.Clear();
            listView1.Clear();

            

            imageListLarge.ColorDepth = System.Windows.Forms.ColorDepth.Depth32Bit;
            imageListLarge.ImageSize = new System.Drawing.Size(100, 148);

            for (index = 1; index < dt.Rows.Count; index++)
            {
                using (Image temp = Image.FromFile(dt.Rows[index]["CoverFile"].ToString()))
                {
                    imageListLarge.Images.Add(index.ToString(), (Image)temp.Clone());
                }
                GC.Collect();
            }

            listView1.LargeImageList = imageListLarge;

            for (index = 1; index < dt.Rows.Count; index++)
            {
                ListViewItem listItem = new ListViewItem(dt.Rows[index]["MovieTitle"].ToString(), index.ToString());
                listView1.ContextMenuStrip = mnu;
                listView1.Click += new EventHandler(InfoClick);
                listView1.DoubleClick += new EventHandler(SelectMovie);
                listView1.Items.Add(listItem);
            }
        }

So when I double click an item in the listview it should start Selectmovie or if I click once start InfoClick. The problem is, these 2 event get both triggered multiple times when I call them. How can I prevent this?

Tnx

KFredje

[tlhIn`toq]

Lines 35 and 36.
You are subscribing multiple times. Every time your loop executes you are subscribing again. That's why you raise the event so many times: Once for each time you subscribe.

These tutorials should help.

Bulding an application - Part 1
Building an application - Part 2
Quick and easy custom events
[

[tlhIn`toq]

Also, your method.. YOu named the method "ListView()" ??
I don't recommend this. Naming a method the same as an existing component is going to be a real confusion/maintenance nightmare. You aren't paying for the names. Give your methods meaningful names. If the purpose of this method is to setup your ListView then name it "SetUpListView". Naming convention is that methods are named as verbs because they do something.

mnu for your context menu? Same thing. You aren't paying for these names by the character. Why make maintenance hard on you and your team mates? "myContextMenu" is a name anything can figure out.

Line 22 - Image.FromFile(string) is evil. It maintains a constant link with the file on harddrive. You should read the image in, then clone it to a fresh object, then dispose of the .FromFile version so you free up the file on harddrive for moving/deleting/other applications.


Nice job building your menus programmatically though! Most people would do it in designer and have no real understanding. Nice to see you delve deeper.

[KFredje]

Tnx for the reply.

tlhIn`toq, on 19 September 2011 - 03:07 PM, said:

Also, your method.. YOu named the method "ListView()" ??
I don't recommend this. Naming a method the same as an existing component is going to be a real confusion/maintenance nightmare. You aren't paying for the names. Give your methods meaningful names. If the purpose of this method is to setup your ListView then name it "SetUpListView". Naming convention is that methods are named as verbs because they do something.

About this...I don't have a "ListView()" in my form. The only thing I have is a control in my Form which is called "listView1". I'm kind of confused what you mean with this.

Quote

mnu for your context menu? Same thing. You aren't paying for these names by the character. Why make maintenance hard on you and your team mates? "myContextMenu" is a name anything can figure out.

Right, but I know it's that and I'm the only one working on the application, so I thought it was a good name

Quote

Line 22 - Image.FromFile(string) is evil. It maintains a constant link with the file on harddrive. You should read the image in, then clone it to a fresh object, then dispose of the .FromFile version so you free up the file on harddrive for moving/deleting/other applications.

I tried to fix this, but the only thing I could came up with was this:

using (Image temp = Image.FromFile(dt.Rows[index]["CoverFile"].ToString()))
                    {
                        imageListLarge.Images.Add(index.ToString(), (Image)temp.Clone());
                    }
                    GC.Collect();

I also use something similar for a pictureBox:

using (Image temp = Image.FromFile(oldfullpath))
                {
                    pictureBoxCoverPreview.Image = new Bitmap((Image)temp.Clone());
                }
                GC.Collect();

At the pictureBox it works, but not with the listView.

Quote

Nice job building your menus programmatically though! Most people would do it in designer and have no real understanding. Nice to see you delve deeper.

Tnx
I'm doing this learning everything myself, so it's cool to try out new stuff even if I have to ask a lot after searching google of course

[ragingben]

KFredje, on 20 September 2011 - 09:02 AM, said:

About this...I don't have a "ListView()" in my form. The only thing I have is a control in my Form which is called "listView1". I'm kind of confused what you mean with this.

Your method is called ListView() right up there on line 1.

public void ListView()

KFredje, on 20 September 2011 - 09:02 AM, said:

I tried to fix this, but the only thing I could came up with was this:

using (Image temp = Image.FromFile(dt.Rows[index]["CoverFile"].ToString()))
                    {
                        imageListLarge.Images.Add(index.ToString(), (Image)temp.Clone());
                    }
                    GC.Collect();

I also use something similar for a pictureBox:

using (Image temp = Image.FromFile(oldfullpath))
                {
                    pictureBoxCoverPreview.Image = new Bitmap((Image)temp.Clone());
                }
                GC.Collect();

At the pictureBox it works, but not with the listView.

Look into Image.FromStream. You could read the image into a MemoryStream and then set the Image from that. That way there is no link to the file form using the Image.FromFile method.

Agreed with thlIn`toq on the context menus, sometimes stepping away from the designer can really give you an appreciation for what is going on 'behind the scenes'.

This post has been edited by ragingben: 20 September 2011 - 01:23 AM