Question

I have the following code in which I am adding data to a list view but I end up having redundant items in it. Please let me know where am I going wrong

    private void button2_Click(object sender, EventArgs e)
    {
        listView1.Items.Clear();
        StreamReader sr = new StreamReader("C:\\sample.txt");
        string s;
        s = sr.ReadLine();

        while (s != null)
        {
            s = sr.ReadLine();
            var m = Regex.Match(s, @"^([a-zA-Z._]+)@([\d]+)");

            if(m.Success)
            {
                allcont ac = new allcont();
                ac.name = m.Groups[1].Value;
                ac.number = m.Groups[2].Value;
                con.Add(ac);
                foreach (allcont aa in con)
                {
                    ListViewItem i = new ListViewItem(new string[] { aa.name, aa.number });
                    i.Tag = aa;
                    listView1.Items.Add(i);
                }
                s = sr.ReadLine();
            }
        }
        sr.Close();
    }
    contacts con = new contacts();
    public class contacts:List<allcont>
    { 

    }
    public class allcont
    {
        public string name;
        public string number;
    }
}

My sample.txt has this:

wer@123

erty@098

sdf@645

ytu@432

Update: This is the data my list view shows:

name number    
wer  123
wer  123
erty 098
wer  123
erty 098
sdf  645
wer  123
erty 098
sdf  645
wer  123
erty 098
sdf  645
ytu  432
Was it helpful?

Solution

I guess you get something like

wer@123

wer@123
erty@098

wer@123
erty@098
sdf@645

wer@123
erty@098
sdf@645
ytu@432

Reason : you've got a problem here

allcont ac = new allcont();
ac.name = m.Groups[1].Value;
ac.number = m.Groups[2].Value;
con.Add(ac);
foreach (allcont aa in con)
{
     ListViewItem i = new ListViewItem(new string[] { aa.name, aa.number });
     i.Tag = aa;
     listView1.Items.Add(i);
 }

because you do this in the while loop. (you're adding things to the list con at each loop, and then loop this "incremented list" inside the while loop).

So you should move the "inner loop"

foreach (allcont aa in con)
    {
         ListViewItem i = new ListViewItem(new string[] { aa.name, aa.number });
         i.Tag = aa;
         listView1.Items.Add(i);
     }

outside of the while loop (after the sr.Close)

OTHER TIPS

con.Add(ac);
                foreach (allcont aa in con)
                {
                    ListViewItem i = new ListViewItem(new string[] { aa.name, aa.number });
                    i.Tag = aa;
                    listView1.Items.Add(i);
                }

you are iterating full con collection on every match
get rid of the inner loop

allcont ac = new allcont();
ac.name = m.Groups[1].Value;
ac.number = m.Groups[2].Value;
con.Add(ac);

ListViewItem i = new ListViewItem(new string[] { ac.name, ac.number });
i.Tag = aa;
listView1.Items.Add(i);
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top