Polymorphic Dispatch, Open/Closed, Notification Part2
December 4, 2012 Leave a comment
I received plenty of great feedback about this post on polymorphic dispatch, open/closed principle, and notifications. I have made the following changes based on the comments
1) Send() should have the message as its argument so that for each message the collection does not have to be recreated. The local variable that holds the message goes away:
public interface IAlert { void Send(String message); }
And an implementation looks like:
public void Send(String message) { if (String.IsNullOrEmpty(message)) throw new AlertsException("message cannot be null or empty."); SmtpClient smtpClient = new SmtpClient(); MailMessage mailMessage = new MailMessage(_from, _to, _subject, message); smtpClient.Send(mailMessage); }
2) I should not use an abstract base for the mail – because it tired me to a specific implementation (need a TO, FROM, SUBJECT). Also, I should be favoring interfaces over abstract classes. The benefit of reduction of code by using that abstract base is more than offset by the specific implementation and the making it harder to build extensible emails in the future.
3) My Twitter implementation was based on Twitter APIs for 2 years ago. Twitter now requires OAuth. A full blog post about hooking up to Twitter is found here.
4) Even though I am following the Open/Closed Principle (OCP) for notifications, I still have a problem. If I add a new notification to the project, I can add the notification class and AlertFactory does not change (follows OCP). However, the SqlAlerteeProvider does have to change (violation of OCP) to account for the new kind of notification. For example:
public Alertee GetAlertee(Int32 alerteeId) { Alertee alertee = new Alertee(); alertee.AlerteeId = 1; alertee.FirstName = "Test"; alertee.LastName = "Test"; alertee.AlertList.Add(new LocalEmailAlert("Test","Test","Test")); alertee.AlertList.Add(new TwitterAlert()); //Add new kind of Alert here return alertee; }
I then thought about how to add the alert and have the factory not change. I first looked at the Factory Pattern in Design Patterns. This was no help because they too violate the OCP when they create the specific implementations of the subclasses (Creator class on p. 111)
Product* Creator::Create (ProductId id)
{
if(Id == MINE) return new MyProduct;
if(Id==YOURS) return new YourProduct
etc…
}
I also realized that I have been using the words “factory” wrong my Factories are usually Builders. In any event, the builder pattern also creates specific concrete classes and would therefor have to change as new types of alerts are added to the project.
I then realized that any concrete implementation will violate the OCP. I see two ways out of the problem. I could either make the individual objects responsible for their own creation (bad idea) or use an abstract factory pattern (GoF p.87) (not an all together a bad idea). The abstract factory implementation needs to be alert specific. So if you have TwitterAlert, you have a TwitterAlertFactory that follows the interface of the IFactory pattern. The difference with my current implementation (SqlAlerteeProvider) is about implementing the object, not about implementing the data store.
Assuming this is right, what it means that every new alter that I add to the project is actually 2 classes. Class 1 is the implementation of alert that follows IAlert and Class 2 is the implementation of alert construction that follows IAlertFactory.
I don’t like this solution. The problem is that I still can’t actually create alerts and assign them to users without changing code – now that code can be external (in Main) or in yet a third class that joins Alerts and AlertFactories together. I suppose I can use a DI container but I will still need to alter the .config file and have a redeploy, so I get no practical value-added. Just by moving something higher up in the call tree does not make it follow OCP. The main function will still have to change and be aware of the types of alerts out there.
I think I will go back to the provider model because it follows the MSFT patterns of use. If I add a new alert, the Alert has to be added and the AlertProvider will have to change. Not pure OCP, but closer – and the code is much cleaner than the typical if..then switch…case spaghetti that you typically find in most projects.