I started on working on a new application last week – one that will read data from various sources, analyze the data, and then notify interested parties about the results of the analysis. I started working on the notification piece first. My first premise is that there can be an variety of notification methods and an infinite number of providers of the methods. For example, a person can receive a notification via an email, a text, a phone call, an IM, an audio signal from their phone app, Twitter, Skype, Facebook, etc… Each of these methods can be served up by a host of providers. Since these providers change so quickly and new mechanisms arrive (and quickly become old mechanisms), it makes sense that this part of my application should be extensible as possible.
After trying a variety of scenarios and objects, I decided to concentrate on the core Interface – which is the Alert. I created an interface like so:
public interface IAlert
{
void Send();
}
I then created a class called Alertee that contains a collection of IAlerts with some other properties:
public class Alertee
{
public Alertee()
{
this.AlertList = new List<IAlert>();
}
public Int32 AlerteeId { get; set; }
public String FirstName { get; set; }
public String LastName { get; set; }
public List<IAlert> AlertList { get; set; }
}
Notice the Command/Query separation. This class is a data structure that only has public properties. I initially toyed with calling the Alertee “User” but “User” is such an overused word – most every implementation of IAlert already has a “User” class – and I am a big believer of domain-unique language (and I hate fully-qualifying my Instances) so I chose the less ambiguous if-not-a-real-word: “alertee”.
I then created an AlertFactory (a command class) like so:
public class AlertFactory
{
public void SendAlerts(List<Alertee> alertees)
{
foreach (Alertee alertee in alertees)
{
foreach (IAlert alert in alertee.AlertList)
{
alert.Send();
}
}
}
}
Notice that by using the IAlert interface, the dependency is inverted and that Alert Factory delegates responsibility of the actual alert implementation via calling the alert.Send().
With the interface, the POCO, and the Factory class ready, I then started implementing the different kinds of alerts. My first stop was an email alert from the local SMTP server. I coded this class like so:
public class LocalEmailAlert: IAlert
{
private String _from = String.Empty;
private String _to = String.Empty;
private String _subject = String.Empty;
private String _body = String.Empty;
public LocalEmailAlert(String from, String to, String subject, String body)
{
if (String.IsNullOrEmpty(from))
throw new AlertsException("from cannot be null or empty.");
if (String.IsNullOrEmpty(to))
throw new AlertsException("to cannot be null or empty.");
if (String.IsNullOrEmpty(subject))
throw new AlertsException("message cannot be null or empty.");
if (String.IsNullOrEmpty(body))
throw new AlertsException("message cannot be null or empty.");
_to = to;
_from = from;
_subject = subject;
_body = body;
}
public void Send()
{
SmtpClient smtpClient = new SmtpClient();
MailMessage mailMessage = new MailMessage(_from, _to, _subject, _body);
smtpClient.Send(mailMessage);
}
}
A couple of things to note. All of the values that the SMTP needs to use are passed in via the constructor (constructor injection) and these values are stored in private fields. Also, the values are verified in the constructor and a generic exception is thrown.
I then went to one of the million email providers out there and picked one at random: MailJet. I then coded up a MailJetEmailProvider like so:
public class MailJetEMailAlert: IAlert
{
private String _mailServerName = "in.mailjet.com";
private Int32 _mailServerPort = 465;
private String _apiKey = String.Empty;
private String _secretyKey = String.Empty;
private String _from = String.Empty;
private String _to = String.Empty;
private String _subject = String.Empty;
private String _body = String.Empty;
public MailJetEMailAlert(String from, String to, String subject, String body)
{
if (String.IsNullOrEmpty(from))
throw new AlertsException("from cannot be null or empty.");
if (String.IsNullOrEmpty(to))
throw new AlertsException("to cannot be null or empty.");
if (String.IsNullOrEmpty(subject))
throw new AlertsException("message cannot be null or empty.");
if (String.IsNullOrEmpty(body))
throw new AlertsException("message cannot be null or empty.");
_to = to;
_from = from;
_subject = subject;
_body = body;
}
public void Send()
{
MailMessage mailMessage = new MailMessage();
mailMessage.From = new MailAddress(_from);
mailMessage.To.Add(new MailAddress(_to));
mailMessage.Body = _body;
mailMessage.Subject = _subject;
SmtpClient client = new SmtpClient(_mailServerName, _mailServerPort);
client.EnableSsl = true;
client.Credentials = new NetworkCredential(_apiKey, _secretyKey);
client.Send(mailMessage);
}
}
Notice that the constructor is exactly the same as the local email implementation. The Mail-Jet specific code/fields are assigned locally and are unique only to the Mailjet class. I suppose that I could pass those elements in via the constructor and then keep the values in the .config file – but that didn’t seem right to me. I also could access the config file directly from this class but that introduces an unneeded dependency – now this class has to worry about the config file (see if it exists, if the section is there, etc…) which is another reason that the class has to change – a violation of the Single Responsibility Principle. The more I code, the more I think that the config file should be accessed only once and in 1 place (in Main) and those values are then passed into the classes that need it. That is a blog post for another time – and certainly is against what Microsoft has recommended for many years.
In any event, I don’t think that having the MailJetspecific code embedded into the class is a bad idea in fact I think it is a good idea. If these values changes, this class has to be recompiled and redeployed (so what, this isn’t java where the calling assembly has to be recompiled) compared to having the UI being mail-jet aware (via its config) and then having the config file redeployed. I would rather have a cleaner separation of concerns and have a recompile than a muddied SOC and no recompile. Both scenarios need to be redeployed anyway.
Back to Mail, I then realized that the mail classes could derive from an abstract base class like so:
public abstract class MailAlert
{
private String _from = String.Empty;
private String _to = String.Empty;
private String _subject = String.Empty;
private String _body = String.Empty;
public MailAlert(String from, String to, String subject, String body)
{
if (String.IsNullOrEmpty(from))
throw new AlertsException("from cannot be null or empty.");
if (String.IsNullOrEmpty(to))
throw new AlertsException("to cannot be null or empty.");
if (String.IsNullOrEmpty(subject))
throw new AlertsException("message cannot be null or empty.");
if (String.IsNullOrEmpty(body))
throw new AlertsException("message cannot be null or empty.");
_to = to;
_from = from;
_subject = subject;
_body = body;
}
}
The problem is that the derived class still needs to access these private fields – the protected scope solves that:
protected String _from = String.Empty;
protected String _to = String.Empty;
protected String _subject = String.Empty;
protected String _body = String.Empty;
So the next problem is abstract base class does not have a constructor that takes zero arguments which is required. So I added a default empty constructor like so:
public MailAlert():
this(String.Empty,String.Empty,String.Empty,String.Empty)
{
}
It now complies and I removed the duplicative code. The sub-lesson here is that, as rule, don’t start with an abstract class. Start with implementations and if there is lots of repetitive code, the consider refactoring to an abstract class. Your covering unit tests will tell you if your refactor is wrong and you are using unit tests, aren’t you?
I then tackled Text pushing via CDyne. I added a reference to the CDyne Service (I love SOAP) in Visual Studio, received a developer key from CDyne, and wrote the following implementation:
public class CDyneTextAlert: IAlert
{
private String _phoneNumber = String.Empty;
private String _message = String.Empty;
private Guid _licenseKey = new Guid("XXXXXXXXXXXXXXXXXXXXXXX");
public CDyneTextAlert(String phoneNumber, String message)
{
if (String.IsNullOrEmpty(phoneNumber))
throw new AlertsException("phoneNumber cannot be null.");
if (String.IsNullOrEmpty(message))
throw new AlertsException("message cannot be null.");
_phoneNumber = phoneNumber;
_message = message;
}
public void Send()
{
IsmsClient client = new IsmsClient();
SMSResponse response = client.SimpleSMSsend(_phoneNumber, _message, _licenseKey);
}
}
I then added a Twitter push like so:
public class TwitterAlert: IAlert
{
private String _twitterUri = "http://twitter.com/statuses/update.json";
private String _twitterUserId = String.Empty;
private String _twitterPassword = String.Empty;
private String _message = String.Empty;
public TwitterAlert(String message)
{
if (String.IsNullOrEmpty(message))
throw new AlertsException("message cannot be null.");
this._message = message;
}
public void Send()
{
HttpWebRequest request = (HttpWebRequest)HttpWebRequest.Create(_twitterUri);
request.Credentials = new NetworkCredential(_twitterUserId, _twitterPassword);
request.Timeout = 5000;
request.Method = "POST";
request.ContentType = "application/x-www-form-urlencoded";
using (Stream requestStream = request.GetRequestStream())
{
using (StreamWriter streamWriter = new StreamWriter(requestStream))
{
streamWriter.Write(_message);
}
}
WebResponse response = request.GetResponse();
}
}
I then added a Phone notification from CDyne:
public class CDynePhoneAlert: IAlert
{
String _phoneNumber = String.Empty;
String _message = String.Empty;
String _callerId = String.Empty;
String _callerName = String.Empty;
String _voiceId = String.Empty;
String _licenseKey = "XXXXXXXXX";
public CDynePhoneAlert(String phoneNumber, String message, String callerId, String callerName, String voiceId)
{
if (String.IsNullOrEmpty(phoneNumber))
throw new AlertsException("phoneNumber cannot be null.");
if (String.IsNullOrEmpty(message))
throw new AlertsException("message cannot be null.");
if (String.IsNullOrEmpty(callerId))
throw new AlertsException("callerId cannot be null.");
if (String.IsNullOrEmpty(callerName))
throw new AlertsException("callerName cannot be null.");
if (String.IsNullOrEmpty(voiceId))
throw new AlertsException("voiceId cannot be null.");
_phoneNumber = phoneNumber;
_message = message;
_callerId = callerId;
_callerName = callerName;
_voiceId = voiceId;
}
public void Send()
{
PhoneNotifySoapClient client = new PhoneNotifySoapClient("PhoneNotifySoap");
NotifyReturn callReturnValue = client.NotifyPhoneBasic(_phoneNumber, _message, _callerId, _callerName, _voiceId, _licenseKey);
}
}
I noticed that the message variable is being passed into the constructor in every IALert implementation so perhaps that should be added to the interface definition Send(String message) but then realized that each implementer might have its own requirements on the message. For example, Twitter needs to check the number of characters:
public TwitterAlert(String message)
{
if (String.IsNullOrEmpty(message))
throw new AlertsException("message cannot be null.");
if(message.Length > 140)
throw new AlertsException("message is too long for Twitter.");
this._message = message;
}
Also, what happens when the message is no longer a string – perhaps a .mp3 file with an recorded voice message to be sent to the phone? Then the Send() method would have to overload an audioStream argument.
So instead of trying to build some class (or worse, some enum) of the different types messages that are passed in, I left it to the specific implementation. This makes sense of the fluid nature of the distribution mechanisms in today’s technology landscape – the API needs to be as flexible as possible.
So far, I have a solution that follows the Open/Closed principle. As new distribution mechanisms come available, I just need to add a new class that implements the IAlert interface, add it to the Alertee’s Alerts collection, and it implements. No other code needs to change. I then ran into a bump – adding an Alert to the Alertee’s Alerts collection.
I added 1 more class to the project – a Provider that creates alertes with instantiated Alerts. I added a SqlAlerteeProvider figuring I will store each Alertee’s desired Alert mechanism (0 to infinity) in a Sql Server database so it looks like so:
public class SqlAlerteeProvider
{
public Alertee GetAlertee(Int32 alerteeId)
{
return null;
}
public List<Alertee> GetAllAlertees()
{
return null;
}
public void UpdateAlertee(Alertee alertee)
{
}
public void InsertAlertee(Alertee alertee)
{
}
public void DeleteAlertee(Alertee aleertee)
{
}
}
However, I am deferring the implementation of the data store for as long as possible. Who knows, I may find a different place to store the data.
So does application follow the open/closed principle? It does up to the point of the Alertee Provider. If a new alert needs to be created, a new class is added (ok with O/C) AND the GetAlertee code needs to be changed to account for the new provider (new fields in the database, etc..) (not OK with the O/C). I suppose I can dig into making the provider follow the O/C, but that is a task for another day.
The important thing is that the calling application DOESN’T change – It just keeps calling SendAlerts() on the AlertFactory regardless of what else happens.