Friday I attended the .NET BootCamp “NHibernate vs. Entity Framework” in Leipzig and as always it was a pleasure for me being there. Afterwards I had a nice talk with my friend Alexander Groß about the Open/Closed Principle. I didn’t really care about this principle before, but now I think it’s really a nice idea:
“In object-oriented programming, the open/closed principle states "software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification"; that is, such an entity can allow its behaviour to be modified without altering its source code.”
If we follow this principle we get lot’s of small and testable classes. I want to demonstrate this with a simple spam checker for mails.
Let’s say our mail class has only a sender, a recipient, a subject and the mail body:
public class EMail
{
public string Sender { get; set; }
public string Recipient { get; set; }
public string Subject { get; set; }
public string Body { get; set; }
public EMail(string sender, string recipient,
string subject, string body)
{
Sender = sender;
Recipient = recipient;
Subject = subject;
Body = body;
}
}
public enum SpamResult
{
Spam,
Ok,
Unknown
}
Now we want to know if a mail is spam or not. Of course we need some rules and some kind of “rule checker” to decide this. Here is a very naïve implementation for this:
public class RuleChecker
{
public SpamResult CheckMail(EMail mail)
{
var result = TestRule1(mail);
if(result != SpamResult.Unknown)
return result;
result = TestRule2(mail);
if (result != SpamResult.Unknown)
return result;
// …
return SpamResult.Unknown;
}
private SpamResult TestRule1(EMail mail)
{
// I don’t care about the concrete rules
}
private SpamResult TestRule2(EMail mail)
{
// I don’t care about the concrete rules
}
}
It is obvious that this implementation breaks the Open/Closed Principle. Every time someone comes up with a new anti-spam rule or the rule priorities change I have to modify the code in CheckMail(). Another problem here is that I can’t test CheckMail() isolated from the concrete rules.
With the help of the Open/Closed Principle our implementation could look like this:
public interface ISpamRule
{
SpamResult CheckMail(EMail mail);
}
public class RuleChecker
{
private readonly IEnumerable<ISpamRule> _rules;
public RuleChecker(IEnumerable<ISpamRule> rules)
{
_rules = rules;
}
public SpamResult CheckMail(EMail mail)
{
foreach (var rule in _rules)
{
var result = rule.CheckMail(mail);
if (result != SpamResult.Unknown)
return result;
}
return SpamResult.Unknown;
}
}
Now you could easily write isolated UnitTests for RuleChecker.CheckMail() and for every new rule.
You get the your concrete RuleChecker by calling the constructor with a list of rules:
class MyFirstRule : ISpamRule
{
public SpamResult CheckMail(EMail mail)
{
// I don’t care about this
}
}
class MySecondRule : ISpamRule
{
public SpamResult CheckMail(EMail mail)
{
// I don’t care about this
}
}
// …
var ruleChecker =
new RuleChecker(
new List<ISpamRule>
{
new MyFirstRule(),
new MySecondRule(),
// …
});
Alex please correct me if I’m wrong, but I think this is what you had in mind Friday.
As stated before, we end up writing lot’s of very small classes – mostly with only one (public) method. I think this is some kind of functional approach, the only question is how we glue our code entities together. Let’s look at the corresponding F# implementation:
type EMail =
{ Sender: string;
Recipient: string;
Subject: string;
Body: string}
type SpamResult =
| Spam
| OK
| Unknown
let checkMail rules (mail:EMail) =
let rec checkRule rules =
match rules with
| rule::rest ->
match rule mail with
| Unknown -> checkRule rest
| _ as other -> other
| [] –> Unknown
checkRule rules
The signature of checkMail is (EMail -> SpamResult) list -> EMail –> SpamResult, which means it takes a list of rules (as above the order is important) and a EMail and returns the SpamResult. In addition I exchanged the explicit foreach loop with a tail recursion to make it look more functional.
If I want a concrete rule checker I could use partial application:
let myFirstRule mail =
// I don’t care about this
let mySecondRule mail =
// I don’t care about this
// val ruleChecker : (EMail –> SpamResult)
let ruleChecker =
checkMail
[ myFirstRule;
mySecondRule]
As you can see the F# implementation is nearly the same as the C# implementation, just without explicitly wrapping our public method in classes. If we would use Reflector we would see that the F# compiler is building the classes around our functions. One could say if we follow the Open/Closed Principle we come to functional code or the other way around if we write functional code we automatically apply the Open/Closed Principle. I think that’s why I really didn’t care about this before.
Appendix
After thinking about this implementation and the extra type hint (see mail:EMail) I came up with a slightly more generic implementation:
type SpamResult =
| Spam
| OK
let checkRules rules element =
let rec checkRule rules =
match rules with
| rule::rest ->
match rule element with
| None -> checkRule rest
| _ as other -> other
| [] –> None
checkRule rules
Here I deleted the enum value for SpamResult.Unknown and used the standard None option. As a consequence the signature changed to: val checkRules : (‘a -> ‘b option) list -> ‘a -> ‘b option. The function still takes a list of rules and a element and returns a option value. Now the checkRules function works with every kind of rule result and takes arbitrary elements.
Tags: .NET Usergroup Leipzig, C-Sharp, F#, Open/Closed Principle, SOLID
This is absolutely what I had in mind when we talked about OCP! 😉
Comment by Alexander Groß — Monday, 24. August 2009 um 12:04 Uhr
I’m just wondering…
Is there any reason for writing the pattern:
_ as other
instead of simply:
other?
Comment by Diego — Monday, 24. August 2009 um 20:46 Uhr
Hi Diego,
the reason is I simply forgot that this is also valid. 😉
Regards,
Steffen
Comment by Steffen Forkmann — Tuesday, 25. August 2009 um 9:00 Uhr
How about using List.pick or List.tryPick? (F# 1.9.6.16)
type SpamResult = Spam | OK
rule’s type: EMail -> SpamResult option
let checkRules rules email = List.pick ((|>) email) rules
Comment by SooHyoung Oh — Wednesday, 26. August 2009 um 6:27 Uhr
Hi SooHyoung,
thanks for the hint.
let checkRules rules element =
List.tryPick ((|>) element) rules
Now the generic rule checker is really nice.
Regards Steffen
Comment by Steffen Forkmann — Wednesday, 26. August 2009 um 9:06 Uhr