0

Is there a syntactic sugar or more elegant way to refactor the following code:

foreach (var recipient in subscription.Recipients)
{
    switch (recipient.ReceivingMethod)
    {
        case ReceivingMethod.To:
            mail.To.Add(recipient.EMailAdress);
            break;
        case ReceivingMethod.Copy:
            mail.Copy.Add(recipient.EMailAdress);
            break;
        case ReceivingMethod.BlindCopy:
            mail.BlindCopy.Add(recipient.EMailAdress);
            break;
    }
}
Raul
  • 2,745
  • 1
  • 23
  • 39

1 Answers1

2

423 characters

foreach (var recipient in subscription.Recipients)
{
    switch (recipient.ReceivingMethod)
    {
        case ReceivingMethod.To:
            mail.To.Add(recipient.EMailAdress);
            break;
        case ReceivingMethod.Copy:
            mail.Copy.Add(recipient.EMailAdress);
            break;
        case ReceivingMethod.BlindCopy:
            mail.BlindCopy.Add(recipient.EMailAdress);
            break;
    }
}

311 characters

mail.To.AddRange(subscription.Recipients.Where(c => c.ReceivingMethod == ReceivingMethod.To))
mail.Copy.AddRange(subscription.Recipients.Where(c => c.ReceivingMethod == ReceivingMethod.Copy))
mail.BlindCopy.AddRange(subscription.Recipients.Where(c => c.ReceivingMethod == ReceivingMethod.BlindCopy))

Either way. Though i personally think the former is the more readable for me

Though if you have printable character OCD, maybe the later

Note : I'm not sure a Dictionary brings much to the table though


Update from Comments

Zohar Peled : That's also assuming mail.To, mail.Copy and mail.BlindCopy supports AddRange - I'm not sure that assumption is correct, since System.Net.MailMessage use the MailAddressCollection classes for To, CC and BCC, and that class does not support AddRange

and

GolezTrol : Also note that the second option is not just syntactical sugar. Internally you loop through the recipients 3 times, and each results in an enumerable that is passed to AddRange, instead of one by one to Add. If AddRange is supported, the end result should be the same, though, so it's typically not something to worry about

TheGeneral
  • 79,002
  • 9
  • 103
  • 141
  • 2
    That's also assuming `mail.To`, `mail.Copy` and `mail.BlindCopy` supports `AddRange` - I'm not sure that assumption is correct, since `System.Net.MailMessage` use the `MailAddressCollection` classes for `To`, `CC` and `BCC`, and that class does not support `AddRange`. – Zohar Peled Feb 12 '18 at 07:24
  • 1
    1) it's harder to read 2) you're executing the code all the time! –  Feb 12 '18 at 07:24
  • *"Though if you have printable character OCD, maybe the later"* - Or find a different language to work in. :-) I agree completely that the switch statement is easier to read. Also note that the second option is not just syntactical sugar. Internally you loop through the recipients 3 times, and each results in an enumerable that is passed to `AddRange`, instead of one by one to `Add`. If `AddRange` is supported, the end result should be the same, though, so it's typically not something to worry about. – GolezTrol Feb 12 '18 at 07:25
  • Also No more vote downs! i have a multiple of 5 again for my rep! my OCD has finally gone, or if you do make sure you get 5 of your friends to do it as well – TheGeneral Feb 12 '18 at 07:32
  • Thanks for the suggestions – Raul Feb 12 '18 at 08:03