Nick LaMarca
Nick LaMarca

Reputation: 8188

Alternate way to use String.Split

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

Answers (7)

robrich
robrich

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

Gennady Chernyak
Gennady Chernyak

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

James Johnson
James Johnson

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

Douglas
Douglas

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

Tim S.
Tim S.

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

usr
usr

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

Tamara Wijsman
Tamara Wijsman

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

Related Questions