Optimizing email alert code

Question

To avoid switch statements at multiple places, I wrote the following code. This was based on articles found in the Google search. The object names are not real in the sample below. So please ignore that.

 //somewhere in the code
foreach (IEmailAlertDt alert in EmailAlertFactory.GetEmailAlertDtType(allowedEmails))
{
    if (alert.CheckEmailAlertDt(currReason))
    {                                
         SendEmail(alert.EmailSubject(patRegister.Pat.Id.ToString()), alert.EmailBody(patRegister.Pat.Id.ToString(), patRegister.Organ.Descrip));
         alert.UpdateEmailAlertDt(currReason);
    }
}


[Flags]
enum EmailAlertType
{
    NoAlert=0,
    FirstAlert=1,
    SecondAlert=2,
    ThirdAlert=4
};

static class EmailAlertFactory
{
    private static Dictionary<EmailAlertType, Func<IList<IEmailAlertDt>>> EmailAlertMap = new Dictionary<EmailAlertType, Func<IList<IEmailAlertDt>>>
    {
        {EmailAlertType.FirstAlert,()=> new List<IEmailAlertDt>() {new FisrtEmailAlertDt() } },
        {EmailAlertType.FirstAlert | EmailAlertType.SecondAlert ,()=> new List<IEmailAlertDt>() {new FisrtEmailAlertDt(), new SecondEmailAlertDt() } },
        {EmailAlertType.FirstAlert | EmailAlertType.SecondAlert | EmailAlertType.ThirdAlert,()=> new List<IEmailAlertDt>() {new FisrtEmailAlertDt(), new SecondEmailAlertDt(), new ThirdEmailAlertDt() } }
    };

    public static IList<IEmailAlertDt> GetEmailAlertDtType(EmailAlertType alertType)
    {
        return EmailAlertMap[alertType]();
    }

}


interface IEmailAlertDt
{
    bool CheckEmailAlertDt(PatWaitListStatReason reason);
    void UpdateEmailAlertDt(PatWaitListStatReason reason);
    string EmailBody(string patID, string Organ);
    string EmailSubject(string patID);
}

class FisrtEmailAlertDt : IEmailAlertDt
{
    public bool CheckEmailAlertDt(PatWaitListStatReason reason)
    {
        if (reason.EmailAlertFirstDate == null)
            return true;
        else
            return false;
    }

    public void UpdateEmailAlertDt(PatWaitListStatReason reason)
    {
        if (reason.EmailAlertFirstDate == null)
            reason.EmailAlertFirstDate = DateTime.Today;
    }

    public string EmailBody(string patID, string Organ)
    {            
            return String.Format(PatOnHoldSuspension.emailBody, PatOnHoldSuspension.FIRST_ALERT_DAYS, patID, Organ);             
    }

    public string EmailSubject(string patID)
    {
        return String.Format(PatOnHoldSuspension.emailSubject, PatOnHoldSuspension.FIRST_ALERT_DAYS,patID);
    }       

}

class SecondEmailAlertDt : IEmailAlertDt
{
    public bool CheckEmailAlertDt(PatWaitListStatReason reason)
    {
        if (reason.EmailAlertSecondDate == null)
            return true;
        else
            return false;
    }

    public void UpdateEmailAlertDt(PatWaitListStatReason reason)
    {
        if (reason.EmailAlertSecondDate == null)
            reason.EmailAlertSecondDate = DateTime.Today;

    }

    public string EmailBody(string patID, string Organ)
    {
        return String.Format(PatOnHoldSuspension.emailBody, PatOnHoldSuspension.SECOND_ALERT_DAYS, patID, Organ);
    }

    public string EmailSubject(string patID)
    {
        return String.Format(PatOnHoldSuspension.emailSubject, PatOnHoldSuspension.SECOND_ALERT_DAYS, patID);
    }

}

class ThirdEmailAlertDt : IEmailAlertDt
{
    public bool CheckEmailAlertDt(PatWaitListStatReason reason)
    {
        if (reason.EmailAlertThirdDate == null)
            return true;
        else
            return false;
    }

    public void UpdateEmailAlertDt(PatWaitListStatReason reason)
    {
        if (reason.EmailAlertThirdDate == null)
            reason.EmailAlertThirdDate = DateTime.Today;

    }

    public string EmailBody(string patID, string Organ)
    {
        return String.Format(PatOnHoldSuspension.emailBody120, PatOnHoldSuspension.THIRD_ALERT_DAYS, patID, Organ);
    }

    public string EmailSubject(string patID)
    {
        return String.Format(PatOnHoldSuspension.emailSubject120, patID);
    }

}

The things that bother me is that there are different small methods like CheckEmailAlertDt, with minor difference. In the first date1 is used and in seconds one date2 is used. Is there any way I can further optimise/streamline this code?

Answer 1

1. I really like your use of a Dictionary with Functions, that is one of my favorite things to do in C# for some reason.

2. Your methods that look like this

   public bool CheckEmailAlertDt(PatWaitListStatReason reason)
   {
       if (reason.EmailAlertFirstDate == null)
           return true;
       else
           return false;
   }
   

   could be written much simpler as

   public bool CheckEmailAlertDt(PatWaitListStatReason reason)
   {
       return reason.EmailAlertFirstDate == null;
   }
   

3. Use switch Statements.... Or not, if not... this is probably as optimized the code is going to get. Here is my rendition of what your code will look like if using switches instead. I just switched the regular flag enums, although you'll probably want to bitwise OR them with the rest of the values as you were doing in your OP

   class EmailAlertDt
   {
       public EmailAlertType EmailAlertType { get; private set; }
   
       public EmailAlertDt(EmailAlertType emailAlertType)
       {
           EmailAlertType = emailAlertType;
       }
   
       public bool CheckEmailAlertDt(PatWaitListStatReason reason)
       {
           switch (EmailAlertType)
           {
               case EmailAlertType.FirstAlert: return reason.EmailAlertFirstDate == null;
               case EmailAlertType.SecondAlert: return reason.EmailAlertSecondDate == null;
               case EmailAlertType.ThirdAlert: return reason.EmailAlertThirdDate == null;
               case EmailAlertType.NoAlert: return false;
               default: throw new SomeThingException("My alerts type is all messed up =\\ ");
           }
       }
   
       public void UpdateEmailAlertDt(PatWaitListStatReason reason)
       {
           switch (EmailAlertType)
           {
               case EmailAlertType.FirstAlert: _updateEmailAlertDt(reason.EmailAlertFirstDate); break;
               case EmailAlertType.SecondAlert: _updateEmailAlertDt(reason.EmailAlertSecondDate); break;
               case EmailAlertType.ThirdAlert: _updateEmailAlertDt(reason.EmailAlertThirdDate); break;
               case EmailAlertType.NoAlert: break;
               default: throw new SomeThingException("My alerts type is all messed up =\\ ");
           }
       }
   
       private void _updateEmailAlertDt(ref DateTime emailAlertDate)
       {
           if (emailAlertDate == null)
               emailAlertDate = DateTime.Today;
       }
   
       public string EmailBody(string patID, string Organ)
       {
           object days;
   
           switch (EmailAlertType)
           {
               case EmailAlertType.FirstAlert: days = PatOnHoldSuspension.FIRST_ALERT_DAYS; break;
               case EmailAlertType.SecondAlert: days = PatOnHoldSuspension.SECOND_ALERT_DAYS; break;
               case EmailAlertType.ThirdAlert: days = PatOnHoldSuspension.THIRD_ALERT_DAYS; break;
               case EmailAlertType.NoAlert: throw new SomeThingException("Halp");
               default: throw new SomeThingException("My alerts type is all messed up =\\ ");
           }
           return String.Format(PatOnHoldSuspension.emailBody120, days, patID, Organ);
       }
   
       public string EmailSubject(string patID)
       {
           return String.Format(PatOnHoldSuspension.emailSubject120, patID);
       }
   }