Reputation: 8188
The below code looks in a string composed of emails seperated by commas and adds an email if it isnt already in the result collection which is also of type string.
string [] oEmails=orderEmails.Split(',');
string[] partEmails= part[Constants.Emails].ToString().Split(',');
foreach(string email in oEmails)
{
if(!partEmails.Contains(email))
{
part[Constants.Emails] += "," + email;
}
}
Is this the best approach to write this logic? I works fine, but I was wondering maybe if there is a way I can consolidate this logic into a lambda expression?
What bothers me is I am doing nothing with these arrays but splitting the input string. They serve no other purpose.
Upvotes: 0
Views: 868
Reputation: 13205
Try this:
part[Constants.Emails] = string.Join( ",", (
from e in (part[Constants.Emails].ToString() + "," + orderEmails).Split(',')
where !string.IsNullOrEmpty( e )
select e
).Distinct().ToArray() );
It's less readable, but likely runs much faster.
EDIT: Most other solutions (including the question) use string concatination. StringBuilder is much, much better for this.
EDIT: Storing this data as List or a hash table is much better than concatinating the resulting array/list into a comma-delimited string and parsing it back out every time, say nothing to getting the wrong answer if one of your strings had a legitimate comma in it.
Upvotes: 0
Reputation: 26
Maybe you can try
var result = oEmails.Select((x, i) => oEmails[i] == partEmails[i] ? part[Constants.Emails] :
partEmails.Contains(x) ? part[Constants.Emails] : -1).ToArray();
Upvotes: 0
Reputation: 46047
You should be able to consolidate the logic like this:
oEmails.Where(partEmails.Contains).Select(e => partEmails[Constants.Emails] += string.Format(",{0}", e));
Upvotes: 0
Reputation: 54877
This way avoids you the part[Constants.Emails].Split(',')
by performing lookups directly within the string:
string[] oEmails = orderEmails.Split(',');
string partEmails = part[Constants.Emails];
foreach (string email in oEmails)
{
bool index = partEmails.IndexOf(email);
bool isAlreadyPresent = index != -1 &&
(index == 0 || partEmails[index - 1] == ',') &&
(index + email.Length == partEmails.Length || partEmails[index + 1] == ',');
if (!isAlreadyPresent)
{
partEmails += "," + email;
}
}
part[Constants.Emails] = partEmails;
Upvotes: 0
Reputation: 56536
Can you use a HashSet
instead of a single large string
? This will only add a value to the list if it does not already exist in the list. E.g.
part[Constants.Emails] = new HashSet<string>();
foreach (var email in oEmails)
part[Constants.Emails].Add(email);
If you need easier conversions to/from one large string, you can make a class, e.g.
class Emails : HashSet<string>
{
public Emails(string concatenatedList)
: base(concatenatedList.Split(','))
{
}
public override string ToString()
{
return string.Join(",", this);
}
}
Upvotes: 3
Reputation: 171178
part[Constants.Emails] =
string.Join(",", part[Constants.Emails].Split(',').Union(orderEmails.Split(',')).ToArray());
Besides that, you are storing the emails the wrong way. Don't store them as a comma separated string, store them as a List. That way you don't have to parse them every time you modify the "collection".
Upvotes: 3
Reputation: 12348
part[Constant.Emails] += oEmails.Where(email => !partEmails.Contains(email))
.Aggregate(b, (current, email)
=> current + ("," + email));
First, this selects the emails that aren't in there yet (first line) then it aggregates them into a string of a few ,mail
parts (second and third line). Then it adds that back to the string (first line).
Upvotes: 0