Reputation: 1153
I am learning programming and my problem is that I have a bunch of objects and I want to add these objects to a list only if the list does not already contain that object. Secondly if the object is already contained I want to ignore that object and add the next one instead. I think I have got the first part working just need some help with part two. Many thanks.
PartyGroup partyGroup = new PartyGroup();
using (AseDataReader reader = command.ExecuteReader())
{
while (reader.Read())
{
if (!myPartyGroupList.Contains(partyGroup))
{
partyGroup.PartyGroupID = Convert.ToInt32(reader["party_group_id"]);
partyGroup.PartyGroupName = reader["party_group_name"].ToString();
partyGroup.PersonList = myPersonList;
myPartyGroupList.Add(partyGroup);
}
else
{
//??
}
}
}
Upvotes: 5
Views: 30074
Reputation: 133975
You might consider using a Hashset<PartyGroup>
to populate, and then convert it to a list. If you have a large number of items, that's going to be significantly faster than checking the list for every item.
Hashset<PartyGroup> pgHash = new Hashset<PartyGroup>();
using (AseDataReader reader = command.ExecuteReader())
{
while (reader.Read())
{
PartyGroup pg = new PartyGroup();
partyGroup.PartyGroupID = Convert.ToInt32(reader["party_group_id"]);
partyGroup.PartyGroupName = reader["party_group_name"].ToString();
partyGroup.PersonList = myPersonList;
// Add won't add an item if it already exists in the collection.
pgHash.Add(partyGroup);
}
}
// Now convert the result to a list.
myPartyGroupList = pgHash.ToList();
If your PartyGroup
class doesn't implement IEquatable<PartyGroup>
, you'll have to provide an equality comparer. The following should work:
public class PartyGroupComparer:IEqualityComparer<PartyGroup>
{
public bool Equals(PartyGroup g1, PartyGroup g2)
{
return g1.PartyGroupId.Equals(g2.PartyGroupId);
}
public int GetHashCode(PartyGroup g)
{
return g.PartyGroupId;
}
}
And then your initialization becomes:
IEqualityComparer<PartyGroup> pgComparer = new PartyGroupComparer();
HashSet<PartyGroup> pgHash = new HashSet<PartyGroup>(pgComparer);
An alternative to the HashSet
, as somebody else pointed out, is the Dictionary
. That will prevent you from having to do the equality comparer. You'll still have to convert to a list when you're done. But that's pretty easy:
Dictionary<int, PartyGroup> dict = new Dictionary<int, PartyGroup>();
// populate dictionary as suggested in other answer
// now convert values to a list.
myPartyGroupList = dict.Values.ToList();
Upvotes: 1
Reputation: 15167
First, you are checking if the object instance is in the collection, but you are only creating one instance once (outside the while loop). Therefore, when you check if !myPartyGroupList.Contains(partyGroup)
, it will return false the first time, so you will add the obj to the collection and then it will return false every time.
I would use a Dictionary using an Id property as the Dictionary key.
like this:
Dictionary <int,PartyGroup> myPartyGroupList = new Dictionary <int,PartyGroup>();
using (AseDataReader reader = command.ExecuteReader())
{
while (reader.Read())
{
int id=Convert.ToInt32(reader["party_group_id"]);
if (!myPartyGroupList.ContainsKey( id ))
{
PartyGroup partyGroup = new PartyGroup();
partyGroup.PartyGroupID = id;
partyGroup.PartyGroupName = reader["party_group_name"].ToString();
partyGroup.PersonList = myPersonList;
myPartyGroupList.Add(id, partyGroup); // key, value? check param order here
}
}
}
Upvotes: 0
Reputation: 8991
When comparing it is better to use comparison with respect to a identifier, which in your case is the PartyGroupId. If you use contains then the default overload of Contains() then the hash value of the object in the list is used for comparison.
So rather than leave the comparison to .NET you can create a custom IEqualityComparer implementation or use Linq's Where clause as follows.
using (AseDataReader reader = command.ExecuteReader())
{
while (reader.Read())
{
int groupId = Convert.ToInt32(reader["party_group_id"]);
if (partyGroupsList.Where(partyGroup => partyGroup.PartyGroupID == groupId).Any() == false)
{
PartyGroup newPartyGroup = new PartyGroup()
{
PartyGroupID = groupId,
PartyGroupName = reader["party_group_name"].ToString(),
PersonList = myPersonList
};
partyGroupsList.Add(newPartyGroup);
}
// If object already exists in the list then do not add, continue
// to the next row.
}
}
Another suggestion is to rename to PartyGroup class members as:
class PartyGroup
{
public int ID { get; set; }
public string Name { get; set; }
public IList PersonList { get; set; }
}
Upvotes: 3
Reputation: 126804
Your code has some issues. First, you're reusing the same object for each of your iterations.
Consider
List<Foo> foos = new List<Foo>();
Foo foo = new Foo();
foo.Bar = "Alpha";
foos.Add(foo);
foo.Bar = "Beta";
foos.Add(foo);
You will note that your list will have 2 items but they will reference the same object. If you iterate over the list and check Bar
, each will return "Beta"
.
You want to create a new Foo
for each item.
List<Foo> foos = new List<Foo>();
Foo foo = new Foo();
foo.Bar = "Alpha";
foos.Add(foo);
Foo anotherFoo = new Foo();
anotherFoo.Bar = "Beta";
foos.Add(anotherFoo);
In looping terms, that basically just means create the object inside the loop instead of outside.
while (someCondition)
{
Foo foo = new Foo();
// do your work, populate the object, etc.
// then check contains
if (!myList.Contains(foo))
myList.Add(foo);
}
Regarding checking to see if the collection already contains the object, have you properly overriden Equals
and GetHashCode
? When dealing the classes, the default behavior is to simply check if the object references are equal. If you're concerned about the values the objects are encapsulating, then you need to provide the logic for that yourself. In your class, you need to override the Equals
and GetHashCode
methods to implement your desired method of determining equality.
class Foo
{
public string Bar { get; set; }
public override int GetHashCode()
{
return this.Bar.GetHashCode();
}
public override bool Equals(object other)
{
Foo otherFoo = other as Foo;
if (otherFoo == null)
return false;
else
return this.Bar == otherFoo.Bar;
}
}
Now when Contains
seeks to determine if the object is already in the list, it will do so based upon the the values contained in the object rather than the memory reference.
Upvotes: 4
Reputation: 8172
Try this
while (reader.Read())
{
partyGroup.PartyGroupID = Convert.ToInt32(reader["party_group_id"]);
partyGroup.PartyGroupName = reader["party_group_name"].ToString();
partyGroup.PersonList = myPersonList;
if (!myPartyGroupList.Contains(partyGroup))
{
myPartyGroupList.Add(partyGroup);
}
}
Upvotes: 0
Reputation: 4401
You've got the first part down perfect.
Just remove the 'else' clause and your routine will automatically add the next element in the next iteration. Like this:
while (reader.Read())
{
if (!myPartyGroupList.Contains(partyGroup))
{
partyGroup.PartyGroupID = Convert.ToInt32(reader["party_group_id"]);
partyGroup.PartyGroupName = reader["party_group_name"].ToString();
partyGroup.PersonList = myPersonList;
myPartyGroupList.Add(partyGroup);
}
}
Upvotes: 5