Send email in a loop while overcoming the exceptions

Question

This is my code

foreach (String Email in usersList)
{ 
   if(Regex.IsMatch(Email, @"\w+([-+.']\w+)*@\w+([-.]\w+)*\.\w+([-.]\w+)*") 
   {

     SmtpClient smtpClient = new SmtpClient();
     MailMessage message = new MailMessage();
     MailAddress AddressFrom = new MailAddress(emailFrom);
     message.From = AddressFrom;
     MailAddress AddressTo = new MailAddress(Email);
     message.To.Add(Email);
     smtpClient.Send(message);
     message.Dispose();
     smtpClient.Dispose();
   }
}

I need to send an email to all users present in the list. But if an exception occurs, the loop breaks and the rest of emails won't be sent. There is other way I can do this so if an email can't be sent the loop keeps iterating and just ignore the ones that fail?

Thank you!

It is rare to call `.Dispose` directly and be doing things correctly. — user7116, Sep 23 '13 at 17:47
Look into `using` constructs, also you may want to rethink your email regex...... http://stackoverflow.com/questions/201323/using-a-regular-expression-to-validate-an-email-address — Arran, Sep 23 '13 at 17:48

evanmcdonnal · Accepted Answer · 2013-09-23T17:53:47.613

7

Yes, but your code could use a decent amount of work.

using (SmtpClient smtpClient = new SmtpClient())
{

    foreach (String Email in usersList)
    { 
       if(Regex.IsMatch(Email, @"\w+([-+.']\w+)*@\w+([-.]\w+)*\.\w+([-.]\w+)*") 
       {
         using (MailMessage message = new MailMessage())
         {
             try
             {
                 MailAddress AddressFrom = new MailAddress(emailFrom);
                 message.From = AddressFrom;
                 MailAddress AddressTo = new MailAddress(Email);
                 message.To.Add(Email);
                 smtpClient.Send(message);
              }
              catch (Exception e)
              {
                  // log exception and keep going
              }
         }
       }
    }
}

Firstly, stop calling Dispose()! When you get that exception, do you know what happens? Dispose() never gets called. Bring you smtp allocation outside of the loop so it's not re-allocated on every iteration. Then just add a try-catch and suppress the exceptions, log them, write them to the console, whatever.

edited Sep 23 '13 at 17:53

answered Sep 23 '13 at 17:48

evanmcdonnal

46,131
16
104
115

You are missing the `Send` call – Adriano Carneiro Sep 23 '13 at 17:53
Hey @evanmcdonnal thank you so much!! The problem here was me using Dispose(). And off course, I added try catch too. Thank you! – Ana Sampaio Sep 23 '13 at 18:12
Just one last question, should I never use smtpClient.Dispose()? – Ana Sampaio Sep 23 '13 at 18:14
1

@AnaSampaio no, the `using` statement ensures that `Dispose()` is called. That's why I added it and removed the `Dispose()` calls, in general `using` is preferred to calling `Dispose()` directly for reasons along the lines of what I mentioned. Here are a couple of useful links if you're want to learn more about it; http://stackoverflow.com/questions/10984336/net-using-using-blocks-vs-calling-dispose http://msdn.microsoft.com/en-us/library/yh598w02.aspx – evanmcdonnal Sep 23 '13 at 18:44

Adriano Carneiro · Answer 2 · 2013-09-23T17:52:04.443

4

How about using "try catch", a.k.a. Exception Handling Statements?

http://msdn.microsoft.com/en-us/library/0yd65esw(v=vs.110).aspx

You code would look like this:

//you don't need a SmtpClient for each recipient
SmtpClient smtpClient = new SmtpClient();

foreach (String Email in usersList)
{ 
   if(Regex.IsMatch(Email, @"\w+([-+.']\w+)*@\w+([-.]\w+)*\.\w+([-.]\w+)*") 
   {
     using (MailMessage message = new MailMessage()){
         try
         {
            MailAddress AddressFrom = new MailAddress(emailFrom);
            message.From = AddressFrom;
            MailAddress AddressTo = new MailAddress(Email);
            message.To.Add(Email);
            smtpClient.Send(message);
         } 
         catch (Exception ex)
         {
            Console.WriteLine(ex.Message);
         }
     }
   }
}

edited Sep 23 '13 at 17:52

answered Sep 23 '13 at 17:46

Adriano Carneiro

57,693
12
90
123

1

This is *exactly* what try/catch blocks are for. – Ant P Sep 23 '13 at 17:47
1

Correct answer, but an example snippet of code would make it complete. (preferably mentioning not catching "Exception"!) – Sayse Sep 23 '13 at 17:48
He was just waiting for me to write it so that he can copy paste :P – evanmcdonnal Sep 23 '13 at 17:54

score 1 · Answer 3 · edited Oct 06 '13 at 16:52

How about Parallel programming?

Parallel.ForEach(usersList, (string email) =>
{ 
   if(Regex.IsMatch(Email, @"\w+([-+.']\w+)*@\w+([-.]\w+)*\.\w+([-.]\w+)*") 
   {
     SmtpClient smtpClient = new SmtpClient();
     MailMessage message = new MailMessage();
     MailAddress AddressFrom = new MailAddress(emailFrom);
     message.From = AddressFrom;
     MailAddress AddressTo = new MailAddress(Email);
     message.To.Add(Email);
     smtpClient.Send(message);
     message.Dispose();
     smtpClient.Dispose();
   }
});

Exception would be thrown in different threads and one thrown exception wouldn't cause breaking the other tasks. Of course you shouldn't forget to use using statement, which makes you sure that the memory will be disposed correctly. In your solution, when exception is thrown, even if it is caught message.Dispose() and smtp.Dispose() aren't executed.

Send email in a loop while overcoming the exceptions

3 Answers3