Syncing e-mails for multiple accounts

Question

I wrote an email syncing program in my ASP.NET MVC website that syncs e-mails from multiple e-mail accounts to my database.

Each e-mail account has to be independent from other accounts and they have to run asynchronously.

Are there are any issues with my code such as efficiency, bugs or can there be made improvements?

 public void Start()
 {
 new Thread(StartSyncing).Start();
 }
 private void StartSyncing()
 {
 foreach (var emailAccount in _emailAccountRepository.GetList())
 {
 // Create a local copy
 EmailAccount copyEmailAccount = emailAccount;
 // Start a new thread for each email account
 new Thread((x => SyncAccount(copyEmailAccount))).Start();
 }
 }
 private void SyncAccount(EmailAccount emailAccount)
 {
 // Sync forever
 while (true)
 {
 // Set the next syncing date
 _nextSyncingDate = DateTime.Now.AddMinutes(5);
 // Create a fresh start for the database context
 InitRepositories();
 // Connect to the mail server
 using (var emailProvider = new ReceiveEmailProvider(emailAccount.Email,
 emailAccount.Password, emailAccount.Host, emailAccount.Port, emailAccount.Secure))
 {
 // For each email
 for (int emailCounter = emailProvider.GetAmountEmails(); emailCounter > 0; emailCounter--)
 {
 // As long the email is new
 while (IsNewEmail(emailCounter))
 {
 // Get email
 var email = emailProvider.GetEmail(emailCounter, emailAccount.Id);
 // Check if the email isn't spam
 if (!IsSpam(GetEmailDomain(email.FromEmail), emailAccount.User.UserId))
 {
 // Add the new email
 var dbEmail = AddNewEmail(email, emailAccount.Id, emailCounter);
 // Add email to category
 AddEmailToCategory(emailAccount.User.UserId, dbEmail);
 }
 }
 }
 }
 // Set end starting sync date
 _endSyncingDate = DateTime.Now;
 // Get the difference in time between the end and starting date
 var timeDifference = _nextSyncingDate - _endSyncingDate;
 // Sleep if we haven't wait for 5 minutes
 if (_nextSyncingDate > _endSyncingDate)
 {
 // Sleep till the 5 minutes have passed
 Thread.Sleep((int)timeDifference.TotalMilliseconds);
 }
 }
 }

I added the methods that seem relevant to me. If you require more code, let me know!

• BCdotWEB: Why don't you do new ReceiveEmailProvider(emailAccount) instead of new ReceiveEmailProvider(emailAccount.Email, emailAccount.Password, emailAccount.Host, emailAccount.Port, emailAccount.Secure)? Or do you not have control over ReceiveEmailProvider?
• Jamie: @BCdotNET I do have control over the receiveEmailProvider. I am using this method because of testing purposes, everything was hardcoded. I'm changing it now!
• Piotr Kula: As it stands, it works. But ideally you want a way to capture exceptions and restart threads. You could create an interface, and implement the inteface for various accounts types etc but that is allot of thinking and code and debugging, and only feasible if you are creating this to be extensible. Otherwise, put throw exception in random places and see how the process handles it, patch it, publish. Done.

Answer 1

I feel like some of the code you haven't shown has a few leaky abstractions. For example:

using (var emailProvider = new ReceiveEmailProvider(emailAccount.Email,
 emailAccount.Password, emailAccount.Host, emailAccount.Port, emailAccount.Secure))

Every argument you send to RecieveEmailProvider comes from emailAccount. I would add an overload to it's constructor so that you could simply pass in an emailAccount. I think it greatly simplifies the code.

using (var emailProvider = new RecieveEmailProvider(emailAccount))

There's something similar happening here.

var dbEmail = AddNewEmail(email, emailAccount.Id, emailCounter);
// Add email to category
AddEmailToCategory(emailAccount.User.UserId,dbEmail);

Why does the client code go through all the trouble of spelling this out? AddEmailToCategory should know how to get the info it needs from an emailAccount so the dev doesn't have to.