patterncsharpModerate
Comparing two lists
Viewed 0 times
twocomparinglists
Problem
I am getting two different lists of members, then if certain conditions meet, I add more members to the list before converting into an array.
Is there any way this could be improve? I guess we can use Linq and cast but I am not advanced in either of the skills I mentioned.
Is there any way this could be improve? I guess we can use Linq and cast but I am not advanced in either of the skills I mentioned.
List Members = new List();
foreach (SPListItem mItem in GetList(Url).Items)
{
Member m = new Member();
m.ID = mItem.ID;
m.Name = mItem.Title;
m.Company = Utilities.ObjectToStringOrEmpty(mItem[companyCol]);
m.eMail = Utilities.ObjectToStringOrEmpty(mItem[emailCol]);
m.Comment = Utilities.ObjectToStringOrEmpty(mItem[commentCol]);
m.Membership = Utilities.ObjectToStringOrEmpty(mItem[msCol]);
Members.Add(m);
}
if (DateTime.Now am.MembershipStatus == "Active" || am.MembershipStatus == "Pending").ToList();
if (activeMembers != null || activeMembers.Count() > 0)
{
foreach (var am in activeMembers)
{
if (!Members.Any(a => a.eMail.ToLowerInvariant() == am.Email.ToLowerInvariant()))
{
Member m = new Member();
m.Name = am.FirstName + " " + am.LastName;
m.eMail = am.Email;
m.IsVip = true;
Members.Add(m);
}
}
}
}
md.Members = Members.ToArray();Solution
You can shorten the following snippet by using LINQ and a projection:
becomes
If a method doesn't require instance-level information, you should make it
This would make more sense as an
Boolean logic! You mean to use
This will create 2 new string objects every iteration. Instead use
Notice also the
You can also shorten the above block by using
The above is written without any IDE but I think you can figure out the solution to any syntax errors in it.
List Members = new List();
foreach (SPListItem mItem in GetList(Url).Items)
{
Member m = new Member();
m.ID = mItem.ID;
m.Name = mItem.Title;
m.Company = Utilities.ObjectToStringOrEmpty(mItem[companyCol]);
m.eMail = Utilities.ObjectToStringOrEmpty(mItem[emailCol]);
m.Comment = Utilities.ObjectToStringOrEmpty(mItem[commentCol]);
m.Membership = Utilities.ObjectToStringOrEmpty(mItem[msCol]);
Members.Add(m);
}becomes
List Members = GetList(Url).Items.Select(item => new Member {
ID = item.ID,
Name = item.Title}).ToList();
// Do this for every field you're interested invar cd = new MemberManager().GetMoreMembers(Url + "/");If a method doesn't require instance-level information, you should make it
static. It will save you another object allocation and it just makes more sense to write MemberManager.GetMoreMembers() then.am.MembershipStatus == "Active" || am.MembershipStatus == "Pending"This would make more sense as an
enum rather than a string. There's only a limited amount of values that status can be.if (activeMembers != null || activeMembers.Count() > 0)Boolean logic! You mean to use
if (activeMembers != null && activeMembers.Count() > 0)a.eMail.ToLowerInvariant() == am.Email.ToLowerInvariant()This will create 2 new string objects every iteration. Instead use
string.Equals(a.eMail, am.Email, StringComparison.InvariantCultureIgnoreCase);Notice also the
eMail and Email discrepancy.You can also shorten the above block by using
var newMembers = activeMembers.Where(activeMember =>
!Members.Any(member => string.Equals(activeMember.eMail,
member.Email,
StringComparison.InvariantCultureIgnoreCase))
.Select(newMember => new Member {
Name = newMember.FirstName + " " + newMember.LastName,
eMail = newMember.Email,
IsVip = true
});
Members = Members.Concat(newMembers);The above is written without any IDE but I think you can figure out the solution to any syntax errors in it.
Code Snippets
List<Member> Members = new List<Member>();
foreach (SPListItem mItem in GetList(Url).Items)
{
Member m = new Member();
m.ID = mItem.ID;
m.Name = mItem.Title;
m.Company = Utilities.ObjectToStringOrEmpty(mItem[companyCol]);
m.eMail = Utilities.ObjectToStringOrEmpty(mItem[emailCol]);
m.Comment = Utilities.ObjectToStringOrEmpty(mItem[commentCol]);
m.Membership = Utilities.ObjectToStringOrEmpty(mItem[msCol]);
Members.Add(m);
}List<Member> Members = GetList(Url).Items.Select(item => new Member {
ID = item.ID,
Name = item.Title}).ToList();
// Do this for every field you're interested invar cd = new MemberManager().GetMoreMembers(Url + "/");am.MembershipStatus == "Active" || am.MembershipStatus == "Pending"if (activeMembers != null || activeMembers.Count() > 0)Context
StackExchange Code Review Q#107523, answer score: 11
Revisions (0)
No revisions yet.