Design principles and testing – part 2
(Head here for an introduction)
(Head here for part one)
Now let’s look at the SaveClick method
1: protected void SaveClick(object sender, EventArgs e)
2: {
3: var newMainBody = NewMainBody.Text;
4:
5: newMainBody = newMainBody
6: .Replace(Environment.NewLine, "<br />");
7:
8: var writablePage = CurrentPage.CreateWritableClone();
9:
10: writablePage.Property["MainBody"].Value = newMainBody;
11:
12: DataFactory.Instance.Save(writablePage, SaveAction.Publish);
13: }
Using our newfound knowledge of the SRP we rewrite this to a MainBodyService, MainBodyFormater and EPiServerRepository.
1: public class MainBodyService
2: {
3: public void FormatAndSave(PageData pageData, string toFormatAndSave)
4: {
5: var formatedText = new MainBodyFormater().Format(toFormatAndSave);
6: new EPiServerRepository().Save(pageData, formatedText);
7: }
8: }
1: public class MainBodyFormater
2: {
3: public string Format(string toFormat)
4: {
5: return toFormat
6: .Replace(Environment.NewLine, "<br />");
7: }
8: }
1: public class EPiServerRepository
2: {
3: public void Save(PageData pageData, string textToSet)
4: {
5: var writablePage = pageData.CreateWritableClone();
6:
7: writablePage.Property["MainBody"].Value = textToSet;
8:
9: DataFactory.Instance.Save(writablePage, SaveAction.Publish, AccessLevel.NoAccess);
10: }
11: }
The only constant in software development…
I’m sure this has never happened to you, but for the case of argument lets pretend that the customer, after looking at what you’ve done so far, wants to change the formatting. In addition to the current formatting they want to replace every instance of the char “@” to the string “(at)” according to some funky WierdBusinessRule (TM).
Because the classes have a defined purpose it’s easy for us to find the MainBodyFormater and change it to apply the new formatting rule:
1: public class MainBodyFormater
2: {
3: public string Format(string toFormat)
4: {
5: return toFormat
6: .Replace(Environment.NewLine, "<br />")
7: .Replace("@", "(at)");
8: }
9: }
So all is well until the customer realize that if you’re logged in they want to append the name of the user to the text. If you’re not logged in it should add “Posted by anon”. Ok, so we continue to edit our method
1: public string Format(string toFormat)
2: {
3: string formattedString = toFormat
4: .Replace(Environment.NewLine, "<br />")
5: .Replace("@", "(at)");
6:
7: if(HttpContext.Current.User.Identity.IsAuthenticated)
8: {
9: formattedString += HttpContext.Current.User.Identity.Name;
10: }
11: else
12: {
13: formattedString += "Posted by anon";
14: }
15:
16: return formattedString;
17: }
You can probably imagine how this method is going to be like after a few more requirement changes. It will likely be quite complex, hard to get a good idea of exactly how the text is formatted and probably quite easy to break.
The open / closed principle (OCP)
So another design principle comes to our aid. The open / closed principle states that “software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification”. So according to this we should be able to change the behavior without changing the code. Case in point, to add the ability to replace the at sign we shouldn’t have to alter code that replaces new line chars. This clearly isn’t the case with out class at the moment since every change in requirements has resulted in us going into the class and adding / changing code.
Refactoring to (better) comply with OCP – approach 1
One alternative is to create a abstract base class for our formatting and then use inheritance to build up functions.
1: public abstract class MainBodyFormaterBase
2: {
3: public abstract string Format(string toFormat);
4: }
1: public class MainBodyFormaterNewLine : MainBodyFormaterBase
2: {
3: public override string Format(string toFormat)
4: {
5: return toFormat
6: .Replace(Environment.NewLine, "<br />");
7: }
8: }
1: public class MainBodyFormaterNewLineAt : MainBodyFormaterNewLine
2: {
3: public override string Format(string toFormat)
4: {
5: return base.Format(toFormat)
6: .Replace("@", "(at)"); ;
7: }
8: }
1: public class MainBodyFormaterNewLineAtPostedBy : MainBodyFormaterNewLineAt
2: {
3: public override string Format(string toFormat)
4: {
5: string formattedString = base.Format(toFormat);
6:
7: if (HttpContext.Current.User.Identity.IsAuthenticated)
8: {
9: formattedString += HttpContext.Current.User.Identity.Name;
10: }
11: else
12: {
13: formattedString += "Posted by anon";
14: }
15:
16: return formattedString;
17: }
18: }
The problem with this approach is that what if we wanted a formatter that didn’t replace newline, replaced @ and printed posted by? Or one that didn’t replace newline nor @ but printed posted by? This quickly turns into building an inheritance scheme that combines every possible scenario which, as you can guess, turns into a class explosion with class names like DoThisIfThatAndThatButNotThatUnlessItsFridayAndDrinksAreFree.
Refactoring to (better) comply with OCP – approach 2
Another alternative here that would solve the class issues from the approach above would be to use the Decorator pattern. Since this is already quite a long post and I didn’t plan on touching Design Patterns in the series I just mention it and move on.
Refactoring to (better) comply with OCP – approach 3
Since it’s apparent that in addition to formatting there’s also cases where some sort of condition decides weather the formatting should occur or not. So let’s build two interfaces to handle the formatting and the rules of whether or not it should format..
1: public interface IMainBodyFormater
2: {
3: string Format(string toFormat);
4: }
5:
6: public interface IShouldFormat
7: {
8: bool ShouldFormat();
9: }
The methods that implements these formatters looks like this (only two shown to save some space)
1: public class MainBodyFormaterNewLine : IMainBodyFormater
2: {
3: public string Format(string toFormat)
4: {
5: return toFormat
6: .Replace(Environment.NewLine, "<br />");
7: }
8: }
1: public class MainBodyFormaterAuthenticated : IMainBodyFormater, IShouldFormat
2: {
3: public string Format(string toFormat)
4: {
5: toFormat += HttpContext.Current.User.Identity.Name;
6: return toFormat;
7: }
8:
9: public bool ShouldFormat()
10: {
11: return HttpContext.Current.User.Identity.IsAuthenticated;
12: }
13: }
We than pass an array of these formatters to the formatting method from our service class.
1: public class MainBodyService
2: {
3: public void FormatAndSave(PageData pageData, string toFormatAndSave)
4: {
5: var formatters = new List<IMainBodyFormater>()
6: {
7: new MainBodyFormaterNewLine(),
8: new MainBodyFormaterAt(),
9: new MainBodyFormaterAuthenticated(),
10: new MainBodyFormaterNotAuthenticated()
11: };
12:
13: var formatedText = new MainBodyFormater().Format(toFormatAndSave, formatters);
14: new EPiServerRepository().Save(pageData, formatedText);
15: }
16: }
And finally the new formatting class
1: public string Format(string toFormat, List<IMainBodyFormater> formatters)
2: {
3: string formattedString = toFormat;
4:
5: foreach(var formatter in formatters)
6: {
7: if(formatter is IShouldFormat)
8: {
9: if(!((IShouldFormat)formatter).ShouldFormat())
10: {
11: continue;
12: }
13: }
14:
15: formattedString = formatter.Format(formattedString);
16: }
17:
18: return formattedString;
19: }
So in order to change the formatting we don’t have to tinker with the formatting method but rather just add a formatter to an array. Pretty slick. We will talk more about additional benefits with this approach later on.
This looks annoyingly complicated, do I have to do this all the time?
Ehm, that’s up to you. Do I follow this all the time? No. As I’ve said before these are principles and guidelines that are there to help you design a more robust and SOLID application, not be in your way or add needless complexity.
One way to look at it is that if you notice that a requirements change from your customer turns out to be a PITA to implement (with your current code structure), you change the code so that a similar change won’t be so hard on you (or your system). During the project these areas of change are often quite easily identified and OCP can make your life (and quicker delivering business value to your customer) a lot easier.
Trying too hard to guess beforehand which those areas will be can be risky. This is related to another principle called YAGNI, You Ain’t Gonna Need It which talks about not writing more code than you currently need to fulfill some business requirement.
Comments