E-store automatic email send, to be run daily

Question

This is a fictional email sending program for a e-store I've done for practice purposes. EmailSenderProgram is a program sending emails to customers. Currently it sends two types of email: "welcome" and "please come back" email. It's supposed to run daily and write a debug log each day if it worked or not.

I'm gonna add more email types later and tried to make it easy for you to later add more emails. I'm Kinda new to programming, but it seems to be working and I would love some improvement tips.

 using System;
    using System.Collections.Generic;

namespace EmailSenderProgram
{
    internal class Program
    {
        /// <summary>
        /// This application is run everyday
        /// </summary>
        /// <param name="args"></param>
        private static void Main(string[] args)
        {
            Console.WriteLine("Send Welcomemail");
            bool success = DoEmailWork();

#if DEBUG

            Console.WriteLine("Send Comebackmail");
            success = DoEmailWork2("CompanyComebackToUs");
#else

            if (DateTime.Now.DayOfWeek.Equals(DayOfWeek.Monday))
            {
                Console.WriteLine("Send Comebackmail");
                success = DoEmailWork2("CompanyComeBackToUs");
            }
#endif

Check if the sending went OK

if (success == true)
                {
                    Console.WriteLine("All mails are sent, I hope...");
                }

Check if the sending was not going well...

if (success == false)
                {
                    Console.WriteLine("Oops, something went wrong when sending mail (I think...)");
                }
                Console.ReadKey();
            }

            /// <summary>
            /// Send Welcome mail
            /// </summary>
            /// <returns></returns>
            public static bool DoEmailWork()
            {
                try
                {   

List<Customer> e = DataLayer.ListCustomers();


for (int i = 0; i < e.Count; i++)
                    {

If the customer is newly registered, one day back in time

if (e[i].CreatedDateTime > DateTime.Now.AddDays(-1))
                        {


System.Net.Mail.MailMessage m = new System.Net.Mail.MailMessage();

                            m.To.Add(e[i].Email);
Add subject
                            m.Subject = "Welcome as a new customer at Company!";
Send mail from company@info.com
                            m.From = new System.Net.Mail.MailAddress("compay@info.com);

m.Body = "Hi " + e[i].Email +
                                     "<br>We would like to welcome you as customer on our site!<br><br>Best Regards,<br>Company Team";
    #if DEBUG

Don't send mails in debug mode, just write the emails in console

Console.WriteLine("Send mail to:" + e[i].Email);
    #else
Create a SmtpClient to our smtphost: yoursmtphost
                        System.Net.Mail.SmtpClient smtp = new System.Net.Mail.SmtpClient("yoursmtphost");
                    smtp.Send(m);
#endif
                    }
                }
                return true;
            }
            catch (Exception)
            {
                return false;
            }
        }

        /// <summary>
        /// Send Customer ComebackMail
        /// </summary>
        /// <param name="v"></param>
        /// <returns></returns>
        private static bool DoEmailWork2(string v)
        {
            try
            {
                //List all customers 
                List<Customer> e = DataLayer.ListCustomers();
                //List all orders
                List<Order> f = DataLayer.ListOrders();

                //loop through list of customers
                foreach (Customer c in e)
                {
                    // We send mail if customer hasn't put an order
                    bool Send = true;
                    //loop through list of orders to see if customer don't exist in that list
                    foreach (Order o in f)
                    {
                        // Email exists in order list
                        if (c.Email == o.CustomerEmail)
                        {
                            //We don't send email to that customer
                            Send = false;
                        }
                    }

                    //Send if customer hasn't put order
                    if (Send == true)
                    {
                        //Create a new MailMessage
                        System.Net.Mail.MailMessage m = new System.Net.Mail.MailMessage();
                        //Add customer to reciever list
                        m.To.Add(c.Email);
                        //Add subject
                        m.Subject = "We miss you as a customer";
                        m.From = new System.Net.Mail.MailAddress("company@info.com");
                        //Add body to mail
                        m.Body = "Hi " + c.Email +
                                 "<br>We miss you as a customer. Our shop is filled with nice products. Here is a voucher that gives you 50 kr to shop for." +
                                 "<br>Voucher: " + v +
                                 "<br><br>Best Regards,<br>Company Team";
#if DEBUG
                        //Don't send mails in debug mode, just write the emails in console
                        Console.WriteLine("Send mail to:" + c.Email);
#else
Create a SmtpClient to our smtphost: yoursmtphost
                        System.Net.Mail.SmtpClient smtp = new System.Net.Mail.SmtpClient("yoursmtphost");

                        smtp.Send(m);
    #endif
                        }
                    }
                    return true;
                }
                catch (Exception)
                {
                    return false;
                }
            }
        }
}

Second class, a file with just the datalayers.

namespace EmailSenderProgram
{
    public class Customer
    {
        public string Email { get; set; }
        public DateTime CreatedDateTime { get; set; }
    }

    public class Order
    {
        public string CustomerEmail { get; set; }
        public DateTime OrderDatetime { get; set; }
    }

    class DataLayer
    {
        /// <summary>
        /// Mockup method for all customers
        /// </summary>
        public static List<Customer> ListCustomers()
        {
            return new List<Customer>()
                       {
                           new Customer(){Email = "mail1@mail.com", CreatedDateTime = DateTime.Now.AddHours(-7)}, 
                           new Customer(){Email = "mail2@mail.com", CreatedDateTime = DateTime.Now.AddDays(-1)}, 
                           new Customer(){Email = "mail3@mail.com", CreatedDateTime = DateTime.Now.AddMonths(-6)}, 
                           new Customer(){Email = "mail4@mail.com", CreatedDateTime = DateTime.Now.AddMonths(-1)}, 
                           new Customer(){Email = "mail5@mail.com", CreatedDateTime = DateTime.Now.AddMonths(-2)},
                           new Customer(){Email = "mail6@mail.com", CreatedDateTime = DateTime.Now.AddDays(-5)}
                       };
        }

        /// <summary>
        /// Mockup method for listing all orders
        /// </summary>
        public static List<Order> ListOrders()
        {
            return new List<Order>()
                       {
                           new Order(){CustomerEmail = "mail3@mail.com", OrderDatetime = DateTime.Now.AddMonths(-6)}, 
                           new Order(){CustomerEmail = "mail5@mail.com", OrderDatetime = DateTime.Now.AddMonths(-2)},  
                           new Order(){CustomerEmail = "mail6@mail.com", OrderDatetime = DateTime.Now.AddDays(-2)}
                       };
        }
    }
}

Answer 1

There are good and bad things about the code, but I will focus on some things that I believe to be bad about the code.

• The EmailSenderProgram namespace should be called EmailSender.
• You make an assumption that the program is run exactly once a day. What happens if it is run twice or not at all on a particular day? You can solve this by including a bool IsWelcomeEmailSent on the Customer class and updating it. Or even better adding a CustomerEmails store.
• This application is run everyday is a bad comment. What happens if the business requirement changes and now it only runs once a week? You should state what the program actually does. For example This application sends emails to customers..
• DoEmailWork and DoEmailWork2 are bad function names. Instead explain the purpose of the function, for example SendWelcomeEmails and SendRetentionEmails.
• If the DoEmailWork function returns false, it will set bool success to false. Then if the DoEmailWork2 function returns true, it will overwrite bool success to true. Use 2 separate variables.
• The use of debug code, #if DEBUG etc, is pretty weird to be honest. You are essentially writing test code into the code itself which is bad practice. Instead you should be using unit testing, injecting all your dependencies so that they can be mocked. This would allow you to mock the datasource as you are already doing, but then when you move to the real database, the code won't need to change. This is a large subject on it's own and beyond the scope of this answer, but research unit testing, dependency injection and mocking.
• Give variables descriptive names. List<Customer> e = DataLayer.ListCustomers(); should be var customers = DataLayer.ListCustomers();. Then instead of for (int i = 0; i < e.Count; i++), use foreach (var customer in customers).
• Which brings me onto inconsistent looping. In DoEmailWork there is a for loop, but in DoEmailWork2 there is a foreach loop. This is confusing.
• There is duplication of the send email code. Instead of building up the MailMessage object in each function, you should have general SendEmail(List<string> recipients, string from, string subject, string body) function that you call from the other functions.

There are other considerations to be made but I think you have enough to start of with. You should also look into Entity Framework so that you can store your data in a database. This will all mesh together with Unit Testing, Dependency Injection and Mocking over time. This is a lot of information but I hope it helps you further improve your code.

EDIT

Here is an example of a possible SendEmail method.

using System.Net.Mail;

public void SendEmail(List<string> recipients, string from, string subject, string body)
{
    var mailMessage = new MailMessage();
    mailMessage.To = string.Join(",", recipients);
    mailMessage.From = new MailAddress(from);
    mailMessage.Subject = subject;
    mailMessage.Body = body;
}

And you can call it like this.

SendEmail(
    new List<string>() { "some@toemail.com", "another@toemail.com" },
    "amazing@fromemail.com",
    "the subject",
    "the super amazing body text and html"
);

Or using your data layer.

SendEmail(
    DataLayer.ListCustomers().SelectMany(customer => customer.Email),
    "amazing@fromemail.com",
    "the subject",
    "the super amazing body text and html"
);

You probably want to add a .Where(customer => customer.CreatedDateTime > DateTime.Now.AddDays(-1)) after ListCustomers() aswell. Then just wrap that function in an appropriately named function.

public void SendWelcomeEmails()
{
    foreach (var customer in DataLayer.ListCustomers()
                .Where(customer => customer.CreatedDateTime > DateTime.Now.AddDays(-1)))
    {
        SendEmail(
            customer.Email,
            "amazing@fromemail.com",
            "the subject",
            $"welcome {customer.Email}, this is the super amazing body text and html"
        );
    }
}