Loop at specific folder in Outlook to look for a specific title

Question

I created a C# console program to loop through a specific folder in Outlook to look for a specific email subject. When I run it on my computer (multi-core CPU), it didn't use up 100% of the CPU resources. But when I test it on the server (single-core CPU), it uses up 100% of the server's CPU resources.

What can I do to optimize the code?

static void Main(string[] args)
{
    try
    {
        string targetFolderName = System.Configuration.ConfigurationSettings.AppSettings["targetFolderName"].ToString().Trim();
        string archiveFolderName = System.Configuration.ConfigurationSettings.AppSettings["archiveFolderName"].ToString().Trim();
        string companyCode = System.Configuration.ConfigurationSettings.AppSettings["companyCode"].ToString().Trim();

        Microsoft.Office.Interop.Outlook.Application outlook = new Microsoft.Office.Interop.Outlook.Application();
        Microsoft.Office.Interop.Outlook.NameSpace nameSpace = outlook.GetNamespace("MAPI");
        Microsoft.Office.Interop.Outlook.MAPIFolder mapiFolderPurchase = (Microsoft.Office.Interop.Outlook.MAPIFolder)nameSpace.GetDefaultFolder(OlDefaultFolders.olFolderInbox).Parent;
        Microsoft.Office.Interop.Outlook.MAPIFolder mapiFolderArchive = (Microsoft.Office.Interop.Outlook.MAPIFolder)outlook.Session.Folders[archiveFolderName].Folders["Inbox"].Parent;

        using (SqlConnection sqlConnection = new SqlConnection(ConfigurationSettings.AppSettings["drawingRequestConnectionString"].Trim()))
        {
            string commandText = "SELECT Regno,UserAccEmail,Rec_date,vendor,approver from SubconJobQ WHERE Status=2 and year(Rec_date)>=2015 ORDER BY Regno DESC";

            SqlCommand sqlCommand = new SqlCommand(commandText, sqlConnection);

            sqlConnection.Open();

            using (SqlDataReader sqlDataReader = sqlCommand.ExecuteReader())
            {
                if (sqlDataReader.HasRows)
                {
                    while (sqlDataReader.Read())
                    {
                        //DateTime recordDate = Convert.ToDateTime(sqlDataReader["Rec_date"].ToString().Trim());
                        //if (recordDate.Year >= 2015)
                        {
                            string[] vendors = sqlDataReader["vendor"].ToString().Trim().Split(';');
                            for (int i = 0; i < vendors.Length; i++)
                            {
                                if (!string.IsNullOrEmpty(vendors[i]))
                                {
                                    string subject = "ASM RFQ: " + vendors[i].Trim() + ", " + sqlDataReader["Regno"].ToString().Trim();

                                    //FIND FOLDER "Sent Email to Vendor"
                                    MAPIFolder vendorFolder = mapiFolderPurchase.Folders["Sent Email to Vendor"];

                                    bool emailFound = false;

                                    //SEARCH EMAIL IN DEFAULT "Sent Email to Vendor" FOLDER
                                    foreach (object item in vendorFolder.Items)
                                    {
                                        MailItem mailItem = item as MailItem;
                                        if (mailItem != null)
                                        {
                                            if (mailItem.Subject != null)
                                            {
                                                if (string.Compare(subject, mailItem.Subject.Trim(), StringComparison.OrdinalIgnoreCase) == 0)
                                                {
                                                    emailFound = true;
                                                    break;
                                                }
                                            }
                                        }
                                    }

                                    //IF EMAIL NOT FOUND IN DEFAULT "Sent Email to Vendor" FOLDER
                                    //SEARCH IN ARCHIVE "Sent Email to Vendor" FOLDER
                                    if (!emailFound)
                                    {
                                        MAPIFolder archiveSentEmailToVendorFolder = mapiFolderArchive.Folders["Sent Email to Vendor"];
                                        foreach (object item in archiveSentEmailToVendorFolder.Items)
                                        {
                                            MailItem mailItem = item as MailItem;
                                            if (mailItem != null)
                                            {
                                                if (mailItem.Subject != null)
                                                {
                                                    if (string.Compare(subject, mailItem.Subject.Trim(), StringComparison.OrdinalIgnoreCase) == 0)
                                                    {
                                                        emailFound = true;
                                                        break;
                                                    }
                                                }
                                            }
                                        }
                                    }

                                    if (!emailFound)
                                    {
                                        ShowMessageInConsole(sqlDataReader["Regno"].ToString().Trim() + " not send to vendor: " + vendors[i].Trim() + " yet!");
                                        //SendAlertEmailToDrawingRequestEmailPersonInCharge("Drawing Request Status Check: ", "");
                                    }

                                }
                            }
                        }
                    }
                }
                else
                {
                    logger.Warn("Not a single Drawing Request is approved!");
                    ShowMessageInConsole("Not a single Drawing Request is approved!");
                    SendAlertEmailToDrawingRequestEmailPersonInCharge("Drawing Request Status Check: " + "Not a single Drawing Request is approved!", "Not a single Drawing Request is approved!");
                }
            }
        }
    }
    catch (System.Exception ex)
    {
        logger.Error("Main Exception", ex);
        ShowErrorMessageInConsole(ex);
    }
    finally
    {
        PressAnyKeyToContinue();
    }
}

Answer 1

You used a using for SqlConnection and SqlDataReader, so why not for SqlCommand?

---

By the time we get to if (sqlDataReader.HasRows) we're already several tabs deep. I'd advise you to move all code inside that if block to a separate method, because it is getting hard to keep an overview.

On a related note: why is your assignment to commandText inside a using inside a try? Define that as a const outside the Main().

---

Please use var so you'll save some screen estate here:

Microsoft.Office.Interop.Outlook.Application outlook = new Microsoft.Office.Interop.Outlook.Application();
Microsoft.Office.Interop.Outlook.NameSpace nameSpace = outlook.GetNamespace("MAPI");
Microsoft.Office.Interop.Outlook.MAPIFolder mapiFolderPurchase = (Microsoft.Office.Interop.Outlook.MAPIFolder)nameSpace.GetDefaultFolder(OlDefaultFolders.olFolderInbox).Parent;
Microsoft.Office.Interop.Outlook.MAPIFolder mapiFolderArchive = (Microsoft.Office.Interop.Outlook.MAPIFolder)outlook.Session.Folders[archiveFolderName].Folders["Inbox"].Parent;

Also, can't Microsoft.Office.Interop.Outlook be moved to a using at the top, e.g. using Microsoft.Office.Interop.Outlook;? That way you'd end up with something much more manageable:

var outlook = new Application();
var nameSpace = outlook.GetNamespace("MAPI");
var mapiFolderPurchase = (MAPIFolder)nameSpace.GetDefaultFolder(OlDefaultFolders.olFolderInbox).Parent;
var mapiFolderArchive = (MAPIFolder)outlook.Session.Folders[archiveFolderName].Folders["Inbox"].Parent;

I notice there are magical strings in there -- "MAPI", "Inbox" -- but in this case it's probably overkill to define them as const since you don't use them elsewhere.

---

All your code is in this Main() of your console application. Why not move it to a class file of its own? It looks much nicer, and offers you much more flexibility.

---

Why are these called repeatedly:

MAPIFolder vendorFolder = mapiFolderPurchase.Folders["Sent Email to Vendor"];
MAPIFolder archiveSentEmailToVendorFolder = mapiFolderArchive.Folders["Sent Email to Vendor"];

I don't see how the contents of these folders are going to change, so why call this over and over again while you're looping through the vendors of each result of an sqlDataReader?

---

You repeatedly do vendors[i].Trim(). If you need such a result multiple times, assign it to a variable. But even better would be to eliminate this when you assign vendors. Matter of fact, why is this a ";"-separated field in your DB? That's some bad design right there.

---

There is no else for if (!string.IsNullOrEmpty(vendors[i])), so reduce nesting by doing this:

if (string.IsNullOrEmpty(vendors[i]))
{
   continue;
}

Same with if (mailItem != null) and if (mailItem.Subject != null).

---

Why did you copy-paste this logic?

foreach (object item in vendorFolder.Items)
{
    MailItem mailItem = item as MailItem;
    if (mailItem != null)
    {
        if (mailItem.Subject != null)
        {
            if (string.Compare(subject, mailItem.Subject.Trim(), StringComparison.OrdinalIgnoreCase) == 0)
            {
                emailFound = true;
                break;
            }
        }
    }
}

foreach (object item in archiveSentEmailToVendorFolder.Items)
{
    MailItem mailItem = item as MailItem;
    if (mailItem != null)
    {
        if (mailItem.Subject != null)
        {
            if (string.Compare(subject, mailItem.Subject.Trim(), StringComparison.OrdinalIgnoreCase) == 0)
            {
                emailFound = true;
                break;
            }
        }
    }
}

This should have been moved to a method:

private bool FindEmail(IEnumerable<MailItem> mailItems)
{
    foreach (var mailItem in mailItems)
    {
        if (mailItem != null)
        {
            if (mailItem.Subject != null)
            {
                if (string.Compare(subject, mailItem.Subject.Trim(), StringComparison.OrdinalIgnoreCase) == 0)
                {
                    return true;
                }
            }
        }
    }
}

Which can be called by:

emailFound = FindEmail(vendorFolder.Items.Cast<MailItem>());

emailFound = FindEmail(archiveSentEmailToVendorFolder.Items.Cast<MailItem>());

I think this could probably be improved upon using LINQ.

---

I'd rethink your whole approach.

First of all I'd dump all that SqlConnection etc. It's 2015, use Entity Framework. For instance, right now the field "Regno" is used in three places: in the SELECT, in the ORDER BY, and it 's used to find a value: sqlDataReader["Regno"].ToString().Trim(). That's three chances of making a typo and three places you need to update if this field ever is renamed.

Secondly, separate you Outlook lookup from your DB query. Right now you're opening a DB connection, execute a query and loop through the results, and in each iteration you're doing at least one Outlook lookup. What you should do is get the db results as entities and loop through those entities.

Answer 2

As was pointed out earlier to use var when setting your variables it would also be a good idea to simplify the class declarations. System.Configuration.ConfigurationSettings for example should be just ConfigurationSettings. To that point you had at least 1 inconsistency with it when you define your sqlConnection.

I would also assume that your compiler was complaining about your use of ConfigurationSettings since it is flagged as Obsolete or Deprecated. You'll have to add a reference to System.Configuration but then you can use the non-obsolete ConfigurationManager in the exact same manner as you are using it now:

var targetFolderName = ConfigurationManager.AppSettings["targetFolderName"].ToString().Trim();
var archiveFolderName = ConfigurationManager.AppSettings["archiveFolderName"].ToString().Trim();
var companyCode = ConfigurationManager.AppSettings["companyCode"].ToString().Trim();

Favor foreach over for. As long as you don't need to know the index of a variable then use foreach. This ...

var vendors = sqlDataReader["vendor"].ToString().Trim().Split(';');
for (int i = 0; i < vendors.Length; i++)
{
    if (string.IsNullOrEmpty(vendors[i])) continue;

    var subject = "ASM RFQ: " + vendors[i].Trim() + ", " + sqlDataReader["Regno"].ToString().Trim();

    //FIND FOLDER "Sent Email to Vendor"
    var vendorFolder = mapiFolderPurchase.Folders["Sent Email to Vendor"];

    var emailFound = false;

    //SEARCH EMAIL IN DEFAULT "Sent Email to Vendor" FOLDER
    emailFound = FindEmail(subject, vendorFolder.Items);

    //IF EMAIL NOT FOUND IN DEFAULT "Sent Email to Vendor" FOLDER
    //SEARCH IN ARCHIVE "Sent Email to Vendor" FOLDER
    if (!emailFound)
    {
        var archiveSentEmailToVendorFolder = mapiFolderArchive.Folders["Sent Email to Vendor"];
        emailFound = FindEmail(subject, archiveSentEmailToVendorFolder.Items);
    }

    if (!emailFound)
    {
        ShowMessageInConsole(sqlDataReader["Regno"].ToString().Trim() + " not send to vendor: " + vendors[i].Trim() + " yet!");
        //SendAlertEmailToDrawingRequestEmailPersonInCharge("Drawing Request Status Check: ", "");
    }
}

Could turn into this...

var vendors = sqlDataReader["vendor"].ToString().Trim().Split(';');
foreach (var vendor in vendors)
{
    if (string.IsNullOrEmpty(vendor)) continue;

    var subject = "ASM RFQ: " + vendor.Trim() + ", " + sqlDataReader["Regno"].ToString().Trim();

    //FIND FOLDER "Sent Email to Vendor"
    var vendorFolder = mapiFolderPurchase.Folders["Sent Email to Vendor"];

    var emailFound = false;

    //SEARCH EMAIL IN DEFAULT "Sent Email to Vendor" FOLDER
    emailFound = FindEmail(subject, vendorFolder.Items);

    //IF EMAIL NOT FOUND IN DEFAULT "Sent Email to Vendor" FOLDER
    //SEARCH IN ARCHIVE "Sent Email to Vendor" FOLDER
    if (!emailFound)
    {
        var archiveSentEmailToVendorFolder = mapiFolderArchive.Folders["Sent Email to Vendor"];
        emailFound = FindEmail(subject, archiveSentEmailToVendorFolder.Items);
    }

    if (!emailFound)
    {
        ShowMessageInConsole(sqlDataReader["Regno"].ToString().Trim() + " not send to vendor: " + vendor.Trim() + " yet!");
        //SendAlertEmailToDrawingRequestEmailPersonInCharge("Drawing Request Status Check: ", "");
    }
}

...which isn't much, but I can then use LINQ because now the compiler will treat vendors as a IEnumerable. This will let me remove one line of code by adding a Where clause. I can simplifiy by making all the values trimmed. End result is a little easier to read IMO:

var regno = sqlDataReader["Regno"].ToString().Trim();
var vendors = sqlDataReader["vendor"].ToString().Split(';')
    .Where(vendor=>!string.IsNullOrEmpty(vendor))
    .Select(vendor=>vendor.Trim());

foreach (var vendor in vendors)
{
    var subject = "ASM RFQ: " + vendor + ", " + regno;

    //FIND FOLDER "Sent Email to Vendor"
    var vendorFolder = mapiFolderPurchase.Folders["Sent Email to Vendor"];

    var emailFound = false;

    //SEARCH EMAIL IN DEFAULT "Sent Email to Vendor" FOLDER
    emailFound = FindEmail(subject, vendorFolder.Items);

    //IF EMAIL NOT FOUND IN DEFAULT "Sent Email to Vendor" FOLDER
    //SEARCH IN ARCHIVE "Sent Email to Vendor" FOLDER
    if (!emailFound)
    {
        var archiveSentEmailToVendorFolder = mapiFolderArchive.Folders["Sent Email to Vendor"];
        emailFound = FindEmail(subject, archiveSentEmailToVendorFolder.Items);
    }

    if (!emailFound)
    {
        ShowMessageInConsole(regno + " not send to vendor: " + vendor + " yet!");
        //SendAlertEmailToDrawingRequestEmailPersonInCharge("Drawing Request Status Check: ", "");
    }
}

This can also stand to have continue used a few more times. Consider the verbiage that you are building in a full context. Say it out loud, then compare what you said to what your code says. With a few renames and continues in place what I said out loud to how the code reads is close:

    var regno = sqlDataReader["Regno"].ToString().Trim();
    var vendors = sqlDataReader["vendor"].ToString().Split(';')
        .Where(vendor=>!string.IsNullOrEmpty(vendor))
        .Select(vendor=>vendor.Trim());

    foreach (var vendor in vendors)
    {
        var subject = "ASM RFQ: " + vendor + ", " + regno;

        //FIND FOLDER "Sent Email to Vendor"
        var sentItems = mapiFolderPurchase.Folders["Sent Email to Vendor"].Items;

        //SEARCH EMAIL IN DEFAULT "Sent Email to Vendor" FOLDER
        if (DoSentItemsHaveSubject(sentItems, subject)) continue;

        //SEARCH IN ARCHIVE "Sent Email to Vendor" FOLDER
        sentItems = mapiFolderArchive.Folders["Sent Email to Vendor"].Items;
        if (DoSentItemsHaveSubject(sentItems, subject)) continue;

        ShowMessageInConsole(regno + " not send to vendor: " + vendor + " yet!");
    }
//...
private static bool DoSentItemsHaveSubject(Items items, string subject)
{
    return items.Cast<MailItem>()
        .Select(item => item.Subject)
        .Where(itemSubject => !string.IsNullOrEmpty(itemSubject))
        .Contains(subject);
}

Often times in code it will be better to tell the user up front what to do when what you expect to find is not found first, then tell them what to do when it is ok. (Null checks are a prime example of this.) IMO this looks and reads a bit better.

if (!sqlDataReader.HasRows)
{
    logger.Warn("Not a single Drawing Request is approved!");
    ShowMessageInConsole("Not a single Drawing Request is approved!");
    SendAlertEmailToDrawingRequestEmailPersonInCharge("Drawing Request Status Check: " + "Not a single Drawing Request is approved!", "Not a single Drawing Request is approved!");
}
while (sqlDataReader.Read())
{
    //DateTime recordDate = Convert.ToDateTime(sqlDataReader["Rec_date"].ToString().Trim());
    //if (recordDate.Year >= 2015)
    {
        var regno = sqlDataReader["Regno"].ToString().Trim();
        var vendors = sqlDataReader["vendor"].ToString().Split(';')
            .Where(vendor => !string.IsNullOrEmpty(vendor))
            .Select(vendor => vendor.Trim());

        foreach (var vendor in vendors)
        {
            var subject = "ASM RFQ: " + vendor + ", " + regno;

            //FIND FOLDER "Sent Email to Vendor"
            var sentItems = mapiFolderPurchase.Folders["Sent Email to Vendor"].Items;

            //SEARCH EMAIL IN DEFAULT "Sent Email to Vendor" FOLDER
            if (DoSentItemsHaveSubject(sentItems, subject)) continue;

            //SEARCH IN ARCHIVE "Sent Email to Vendor" FOLDER
            sentItems = mapiFolderArchive.Folders["Sent Email to Vendor"].Items;
            if (DoSentItemsHaveSubject(sentItems, subject)) continue;

            ShowMessageInConsole(regno + " not send to vendor: " + vendor + " yet!");
        }
    }
}

I can't place my finger on it, but I think that your ADO.NET call to your database is pulling back too much information that could easily be narrowed down with a good where clause.

With those bits in place I think it will help with the debugger side of things, but a better option would be to do as was suggested earlier by encapsulating your work into different classes. By encapsulating you would be able to write some unit-tests and then very quickly determine where your bottle neck is coming from.

How??? Well that is mostly out of the scope of this review, but I will give a hint. Imagine each class having 1 specific purpose. You give it some data and it tells you something about that data. If it is done properly you can narrow down what your bottle neck is.

For instance... say you set your outlook inbox up very specifically, you then feed your "OutlookFolderSeracher" class a very specific vendor to look for (one that you know exists) by either mocking or by passing in a specific string. Well that is all in memory and is VERY fast. If this test takes a long time to finish then you know for certain that the speed lies in your outlook searcher. If that is fast then you can focus on your call to your database. Either way the more you can isolate the easier it is to diagnose where it is slow.

Answer 3

Assuming, that the bottleneck is indeed the foreach loop, you could use the parallel version:

private bool FindEmail(string subject, MAPIFolder folder)
{
    var emailFound = false;
    Parallel.ForEach(folder.Items, (item, state) =>
                                       {
                                           var mailItem = item as MailItem;
                                           if (mailItem == null || mailItem.Subject == null) return;
                                           if (string.Compare(subject, mailItem.Subject.Trim(), StringComparison.OrdinalIgnoreCase) == 0)
                                           {
                                               emailFound = true;
                                               state.Stop();
                                           }
                                       });
    return emailFound;
}

Just make sure to do some actual profiling. I/O related operations usually do not go that well with multithreading. Also it might make more sense to refactor outer for loop instead inner foreach. The choice would depend on the sizes of folder.Items and vendors.