Sending emails using SMTP client

Question

The following code gets the recipient's email address, email subject line and email body from a table. It then creates the emails and sends them to a pickup directory. There is a db connection class that I did not include here, but I invoke it in the code below.

The code below runs fine. I am wondering if its speed can be improved, considering it will be used to send 10,000+ emails.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Text;
using System.Net;
using System.Net.Mail;
using System.Data;
using System.Data.SqlClient;
using System.IO;

namespace SendEmails
{
    class SendEmail
    {
        private class EmailsList
        {
            private class listData
            {
                public string email;
                public string email_header;
                public string email_body;
                public string email_guid;
            }
            public void data()
            {
                SqlDataReader sqlData;
                ArrayList Emaillist = new ArrayList();
                sqlData = new SqlCommand("SELECT email, email_header, email_body, email_guid FROM dbo.vw_emails ", con.openconnection()).ExecuteReader();
                // loop through the emails table and load arraylist
                while (sqlData.Read())
                {
                    listData itemData = new listData();
                    itemData.email = sqlData[0].ToString();
                    itemData.email_header = sqlData[1].ToString();
                    itemData.email_body = sqlData[2].ToString();
                    itemData.email_guid = sqlData[3].ToString();
                    Emaillist.Add(itemData);                
                }               
                sqlData.Close();
                con.closeconnection

                foreach (listData itemData in Eamillist)
                {
                    //SEND EMAIL *****************************/
                    spSendMail(itemData.email, itemData.email_header, "[email protected]", itemData.email_body, itemData.email_guid);
                    //DO THE UPDATE *********************/
                    SqlCommand cmd = new SqlCommand("up_emailLog", con.openconncetion());
                    cmd.CommandType = CommandType.StoredProcedure;
                    cmd.Parameters.AddWithValue("@email_GUID", itemData.email_guid);
                    cmd.CommandTimeout = 0;
                    cmd.ExecuteNonQuery();
                    con.CloseConnection();

                }
            }
        }

        static void Main(string[] args)
        {
            try
            {
               EmailsList sEmails = new EmailsList();
                sEmails.data();                    
            }
            catch (Exception ex)
            {
                SqlCommand cmd = new SqlCommand("INSERT INTO [dbo.Err_Log](exception, insdt) VALUES('" + ex.Message.ToString() + "','" +  "','"+ DateTime.Now + "')", Con.OpenConnection());
                cmd.ExecuteNonQuery();
                con.CloseConnection();                 
            }
        }
        // Send Email Method
        public static void spSendMail(string recipients, string subject, string from, string body, string email_guid)
        {
            try
            {
                using (MailMessage mailMessage = new MailMessage(from, recipients))
                {
                        mailMessage.Subject = subject;
                        mailMessage.Body = body;
                        mailMessage.IsBodyHtml = true;
                        SmtpClient smtpClient = new SmtpClient("xxxxxx.xxxx.zo");
                        smtpClient.UseDefaultCredentials = true;
                        smtpClient.DeliveryMethod = SmtpDeliveryMethod.SpecifiedPickupDirectory;
                        smtpClient.PickupDirectoryLocation = "\\\\exchange_server\\pickup";
                        //Create eml file and send it to pickup directory
                        smtpClient.Send(mailMessage);                               
                }
            }
            catch (Exception ex)
            {
                SqlCommand cmd = new SqlCommand("INSERT INTO [dbo.Err_Log](exception, communication, insdt) VALUES('" + ex.Message.ToString() +"','" + recipients.ToString() + "','" + DateTime.Now + "')", con.OpenConnection());
                cmd.CommandTimeout = 0;
                cmd.ExecuteNonQuery();
                Con.CloseConnection();
            }
        }
    }
}

Answer 1

Have you done any benchmarking?

What part is slower, the database query or the sending of the emails to exchange?

Sending emails is generally the slowest part, but given you are using a pickup directory, that may not be the case here. If it is really slow then you could run several threads in parallel, although this would require significant alterations to your code.

Just curious also how to do you keep track of which emails are sent?

What happens if the process crashes halfway through, will it sent the emails to the users twice?

Keep in mind that your mail server will be the bottleneck at the end of the day. If you want the messages reliably delivered, you will need to have throttling in-place if you are sending to the likes of hotmail, gmail, etc, so regardless of the performance improvements you make in your code, the messages would still be stuck in a queue anyway.

My C# is a bit rusty, but there are a few places where I think you could improve.

In this function, you are recreating the SmtpClient every time, perhaps store it in an instance var instead

public static void spSendMail(string recipients, string subject, string from, string body, string email_guid)

I have made comments in the code below, hopefully this is of some use

    public void data()
    {
        SqlDataReader sqlData;
  //    ArrayList Emaillist = new ArrayList();
        SqlConnection conn;
        SqlCommand cmd;

    // open connection once for the entire function
    conn = con.openconnection();

        sqlData = new SqlCommand("SELECT email, email_header, email_body, email_guid FROM dbo.vw_emails ", conn).ExecuteReader();

    // build cmd once and reuse it
        cmd = new SqlCommand("up_emailLog", conn);
                cmd.CommandType = CommandType.StoredProcedure;
                cmd.CommandTimeout = 0;

        // loop through the emails table and load arraylist

        while (sqlData.Read())
        {
        // why are we storing this in a temporary structure in memory?
    //        listData itemData = new listData();
    //        itemData.email = sqlData[0].ToString();
    //        itemData.email_header = sqlData[1].ToString();
    //        itemData.email_body = sqlData[2].ToString();
    //        itemData.email_guid = sqlData[3].ToString();
    //        Emaillist.Add(itemData);
    //        }
    //        sqlData.Close();
    //        con.closeconnection
    //
    //        foreach (listData itemData in Eamillist)
    //        {
                //SEND EMAIL *****************************/
                spSendMail(sqlData[0].ToString(), sqlData[1].ToString(), "[email protected]", sqlData[2].ToString(), sqlData[3].ToString());
                //DO THE UPDATE *********************/

                // reuse same cmd, just clear parameters
                cmd.Parameters.Clear();
                cmd.Parameters.AddWithValue("@email_GUID", sqlData[3].ToString());
                cmd.ExecuteNonQuery();
        }

        // close connection outside of loop, so we are not constantly opening and closing
        conn.CloseConnection();
    }