{"id":1198,"date":"2011-09-06T00:00:00","date_gmt":"2011-09-06T00:00:00","guid":{"rendered":"https:\/\/test.simple-talk.com\/uncategorized\/writing-maintainable-code\/"},"modified":"2021-05-17T18:36:24","modified_gmt":"2021-05-17T18:36:24","slug":"writing-maintainable-code","status":"publish","type":"post","link":"https:\/\/www.red-gate.com\/simple-talk\/development\/dotnet-development\/writing-maintainable-code\/","title":{"rendered":"Writing Maintainable Code"},"content":{"rendered":"<div id=\"pretty\">\n<p class=\"start\">It&#8217;s an inescapable fact that writing maintainable code is hard. Most projects end up with code that nobody dares touch, caused by a lack of understanding and a fear of error. Before we think about how to avoid such a mess, we have to consider what we mean by maintainable code.<\/p>\n<p>Before we can change any code, we have to understand what the code currently does, and whether it&#8217;s supposed to do that. While there are many ways to achieve this, I think one of the best solutions is a comprehensive suite of automated tests. A good suite of tests can serve as documentation, telling you how the code is supposed to behave, as well as making sure that the code actually has that behaviour. Even better, they can give you the confidence that your code still works after you&#8217;ve made your changes. Unfortunately, some code is impossible to test, so the first question I&#8217;ll try to answer is: how do I write testable code?<\/p>\n<p>Even if we have a suite of tests, changing code is difficult if we have no idea how it works. Therefore, the second question is: how do I write clean, understandable code?<\/p>\n<h2>How do I write testable code?<\/h2>\n<p>If you ask people, &#8220;How do you write untestable code?&#8221;, they&#8217;ll often respond with answers like &#8220;Long, complicated methods&#8221; or &#8220;No clean separation of concerns&#8221;. While these are things to avoid (and we&#8217;ll come back to them), they make code <strong>hard<\/strong> to test, but not <strong>impossible<\/strong> to test. Code tends to be untestable for two main reasons:<\/p>\n<ul>\n<li>Global, <strong>mutable<\/strong> state. In other words, a variable that any code can access, and any code can modify.<\/li>\n<li>Constructing, or otherwise acquiring, services, such as database connections, through static methods (a constructor is effectively a static method).<\/li>\n<\/ul>\n<p>Global variables are fine if it&#8217;s immutable data. If it&#8217;s mutable, then it becomes impossible to consistently set up an identical environment before each test. For instance, say a method uses an incrementing integer to assign unique IDs. Each time we run the method, the assigned ID will be different, making it impossible to test the method with particular IDs.<\/p>\n<p>The solution to the second point is <strong>dependency injection<\/strong>. A class should ask for its dependencies through its constructor rather than acquiring them itself. This allows a test to pass in mocked versions of those dependencies. For instance, let&#8217;s say we have an online booking system for a tattoo parlour. If a user under 18 requests an appointment, we refuse to give them a tattoo. If they&#8217;re 18 or over, we give them the first available appointment. The code might look something like this:<\/p>\n<pre class=\"lang:c# theme:vs2012\">public class Appointments {\r\n\u00a0\u00a0\u00a0 public Result RequestTattooAppointment(User user)\r\n\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 var appointmentBooker = new AppointmentBooker();\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 if (user.Age &gt;= AgeOfConsent)\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 return appointmentBooker.BookFirstAvailableAppointment(user);\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 else\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 return Result.UnderAge();\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0 }\r\n}\r\n<\/pre>\n<p>There&#8217;s no way to test this class in a unit test &#8211; no matter what we do, it will construct an <code>AppointmentBooker<\/code>, which will connect to the database. We don&#8217;t want to have to set up an entire database just to test the logic in this method. Instead, we should ask for an <code>AppointmentBooker<\/code> in the constructor.<\/p>\n<pre class=\"lang:c# theme:vs2012\">public class Appointments {\r\n\u00a0\u00a0\u00a0 private readonly IAppointmentBooker m_AppointmentBooker;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0 \r\n\u00a0\u00a0\u00a0 public Appointments(IAppointmentBooker appointmentBooker)\r\n\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 m_AppointmentBooker = appointmentBooker;\r\n\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0 \r\n\u00a0\u00a0\u00a0 public Result RequestTattooAppointment(User user)\r\n\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 if (user.Age &gt;= AgeOfConsent)\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 return m_AppointmentBooker.BookFirstAvailableAppointment(user);\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 else\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 return Result.UnderAge();\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0 }\r\n}\r\n<\/pre>\n<p>Now we can pass in a mocked instance of <code>IAppointmentBooker<\/code> in our test. For instance, just taking the case of the user being underage:<\/p>\n<pre class=\"lang:c# theme:vs2012\">public class AppointmentsTest\r\n\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0 private readonly IAppointmentBooker m_AppointmentBooker = A.Fake&lt;IAppointmentBooker&gt;();\r\n\u00a0\u00a0\u00a0\u00a0\u00a0 private readonly Appointments m_Appointments;\r\n\u00a0\r\n\u00a0\u00a0\u00a0\u00a0\u00a0 public AppointmentsTest()\r\n\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 m_Appointments = new Appointments(m_AppointmentBooker);\r\n\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\r\n\u00a0\u00a0\u00a0\u00a0\u00a0 public void UnderAgeUsersDoNotGetAnAppointment()\r\n\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 \/\/ Given\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 var user = new User {Age = Appointments.AgeOfConsent - 1};\r\n\u00a0\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 \/\/ When\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 var result = m_Appointments.RequestTattooAppointment(user);\r\n\u00a0\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 \/\/ Then\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 Assert.Equal(Result.UnderAge(), result);\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 A.CallTo(() =&gt; m_AppointmentBooker.BookFirstAvailableAppointment(user)).MustNotHaveHappened();\r\n\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0 }\r\n<\/pre>\n<p>There are some frameworks, such as Google Guice for Java or Ninject for C#, that can automate some parts of dependency injection, saving you from death by a thousand <code>new<\/code>s.<\/p>\n<h2>How do I write clean, understandable code?<\/h2>\n<p>Although there are dozens of useful rules and principles in writing clean code, I think most can be reduced to one of these three:<\/p>\n<ul>\n<li><strong>Optimise for the reader of the code, not the writer<\/strong>. Code is read more than it&#8217;s written. If you optimise for the writer, you&#8217;ll save a few key presses, but the cost to the reader is confusion, frustration, and subtle (and not-so-subtle) bugs.<\/li>\n<li><strong>Don&#8217;t repeat yourself<\/strong> (often abbreviated to DRY). It&#8217;s easier to maintain code if it only exists in one place, and this also ensures consistency. If code is duplicated, there&#8217;s a good chance that you&#8217;ll forget to update one of the copies, meaning bugs you fix in one copy will still be there in the other copy.<\/li>\n<li><strong>Do the simplest, smallest thing you can do to add some value<\/strong>. If you try and guess how you should try and modify the system for all future requirements, you&#8217;re going to get it wrong. Either you&#8217;ll end up rewriting the code you never used, or you&#8217;ll be stuck with code that sort-of does what you actually want it to do.<\/li>\n<\/ul>\n<h3>Optimising for the reader<\/h3>\n<p>The computer doesn&#8217;t care how you write your code, so long as it&#8217;s unambiguous. In other words, write code for humans, not machines. There are plenty of principles you could use, this is just a smattering:<\/p>\n<ul>\n<li>Spend some time carefully thinking about the names for classes and methods. If you find all of your class names end in <code>Helper<\/code>, then you might need to spend a bit more time thinking. Thinking up descriptive names is hard, but being unable to think up a good name is sometimes a sign that your class or method does too much, and actually needs splitting up.<\/li>\n<li>Don&#8217;t use abbreviations. For instance, instead of calling a variable <code>dbc<\/code>, call it <code>databaseConnection<\/code>. The exception is when the abbreviation is well-known, for instance, using <code>html<\/code> as an abbreviation is fine.<\/li>\n<li>Good code should be unsurprising. If you find some code that is surprising or &#8220;clever&#8221;, try to see if you can simplify it, or somehow make it clearer.<\/li>\n<li>Each method should operate at one level of abstraction. A method that deals with high-level concepts in your domain shouldn&#8217;t be doing complicated string manipulation as well. One style of writing code is to make it read like a newspaper article. From reading that method, you get just enough detail to see how it works. If you want more detail on how it works, you can dive into one of the methods being called. Just like a newspaper article, you should be able to stop reading at any point and have an understanding of the overall picture.<\/li>\n<\/ul>\n<h3>Don&#8217;t repeat yourself<\/h3>\n<p>If you ever find yourself copy-and-pasting code, then that&#8217;s a hint that you should pull out the common functionality of the code.<\/p>\n<p>In some languages, it&#8217;s difficult to pull out code that has structural duplication, but operates on different data. However, if you have a language that lets you pass around functions easily, you can pull out that duplication. For instance, say you want to convert a list of strings to a list of integers. You could build a new list of integers, iterate through the list of strings, and add each parsed string to the new list:<\/p>\n<pre class=\"lang:c# theme:vs2012\">var listOfIntegers = new List&lt;Integer&gt;();\r\nforeach (var str in listOfStrings)\r\n{\r\n\u00a0\u00a0\u00a0 listOfIntegers.Add(int.Parse(str));\r\n}\r\n<\/pre>\n<p>Every time you want to convert one list of values to another, the code is identical except for the value conversion. You can pull that duplication out by using map, called <code>Select<\/code> in C#:<\/p>\n<pre class=\"lang:c# theme:vs2012\">var listOfIntegers = listOfStrings.Select(int.Parse);\r\n<\/pre>\n<h3>Do the simplest, smallest thing to add value<\/h3>\n<p>It&#8217;s impossible to guess what code you&#8217;re going to need in the future, even if you think you have a good grasp of the requirements. By taking small steps, you learn about the problem while solving it.<\/p>\n<p>You might notice that always taking small, simple steps doesn&#8217;t always lead to readable code without duplication. It&#8217;s crucial to remember to refactor your code once it&#8217;s working. Once you have code that does what you want in the simplest way, you must then make sure it&#8217;s clean code that you&#8217;d be happy maintaining.<\/p>\n<h3>Example<\/h3>\n<p>Here&#8217;s some code that I reckon could do with a bit of refactoring:<\/p>\n<pre class=\"lang:c# theme:vs2012\">public class ArchiveController : Controller\r\n{\r\n\u00a0\u00a0\u00a0 private readonly ISession m_Session;\r\n\u00a0\r\n\u00a0\u00a0\u00a0 public ArchiveController(ISession session)\r\n\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 m_Session = session;\r\n\u00a0\u00a0\u00a0 }\r\n\u00a0\r\n\u00a0\u00a0\u00a0 public ActionResult Index(FormCollection form)\r\n\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 var sds = form[\"start-date\"];\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 var sd = DateTime.MinValue;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 if (sds != null)\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 if (DateTime.TryParse(sds, out sd))\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 else if (sds == \"now\")\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 sd = DateTime.Now;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 var eds = form[\"end-date\"];\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 var ed = DateTime.MaxValue;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 if (eds != null)\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 if (DateTime.TryParse(eds, out ed))\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 else if (eds == \"now\")\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 ed = DateTime.Now;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 var hss = m_Session.QueryOver&lt;HatSale&gt;().Where(h =&gt; h.DateTime &gt;= sd).Where(h =&gt; h.DateTime &lt;= ed).List();\r\n\u00a0\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 var ahwpm = hss.GroupBy(h =&gt; h.DateTime.Month).Select(g =&gt; new {W = g.Key, Avg = g.Average(s =&gt; s.Hat.Width)});\r\n\u00a0\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 return View(new {Hss = hss, Ahwpm = ahwpm});\r\n\u00a0\u00a0\u00a0 }\r\n}\r\n<\/pre>\n<p>There are clearly some problems with this code:<\/p>\n<ul>\n<li>The class is called <code>ArchiveController<\/code> &#8211; what&#8217;s in this archive?<\/li>\n<li>The variables aren&#8217;t named well, for instance, what does <code>ahwpm<\/code> mean?<\/li>\n<li>The logic that reads <code>start-date<\/code> and <code>end-date<\/code> from the form is duplicated.<\/li>\n<li>What does <code>DateTime<\/code> on a hat sale represent?<\/li>\n<li>There are bits of NHibernate querying and LINQ all on one unreadable line.<\/li>\n<li>The method is long, and you feel you have to read the whole thing to begin to understand what&#8217;s going on.<\/li>\n<li>The method operates at many layers of abstraction &#8211; it&#8217;s parsing strings, accessing the database, and doing some calculations.<\/li>\n<\/ul>\n<p>So, after some refactoring:<\/p>\n<pre class=\"lang:c# theme:vs2012\">public class SalesArchiveController : Controller\r\n{\r\n\u00a0\u00a0\u00a0 private readonly ISession m_Session;\r\n\u00a0\r\n\u00a0\u00a0\u00a0 public SalesArchiveController(ISession session)\r\n\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 m_Session = session;\r\n\u00a0\u00a0\u00a0 }\r\n\u00a0\r\n\u00a0\u00a0\u00a0 public ActionResult Index(FormCollection form)\r\n\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 var startDate = ParseStartDate(form);\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 var endDate = ParseEndDate(form);\r\n\u00a0\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 var hatSales = FetchHatSalesInDateRange(startDate, endDate);\r\n\u00a0\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 var averageHatWidthsPerMonth = CalculateAverageHatWidthsPerMonth(hatSales);\r\n\u00a0\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 return View(new {HatSales = hatSales, AverageHatWidthsPerMonth = averageHatWidthsPerMonth});\r\n\u00a0\u00a0\u00a0 }\r\n\u00a0\r\n\u00a0\u00a0\u00a0 private DateTime ParseStartDate(FormCollection form)\r\n\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 return ParseDateParameterOrNull(form, \"start-date\") ?? DateTime.MinValue;\r\n\u00a0\u00a0\u00a0 }\r\n\u00a0\r\n\u00a0\u00a0\u00a0 private DateTime ParseEndDate(FormCollection form)\r\n\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 return ParseDateParameterOrNull(form, \"end-date\") ?? DateTime.MaxValue;\r\n\u00a0\u00a0\u00a0 }\r\n\u00a0\r\n\u00a0\u00a0\u00a0 private DateTime? ParseDateParameterOrNull(FormCollection form, string key)\r\n\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 var dateString = form[key];\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 if (string.IsNullOrEmpty(dateString))\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 return null;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 DateTime dateTime;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 if (DateTime.TryParse(dateString, out dateTime))\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 return dateTime;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 if (dateString == \"now\")\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 return DateTime.Now;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 return null;\r\n\u00a0\u00a0\u00a0 }\r\n\u00a0\r\n\u00a0\u00a0\u00a0 private IList&lt;HatSale&gt; FetchHatSalesInDateRange(DateTime startDate, DateTime endDate)\r\n\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 return m_Session\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 .QueryOver&lt;HatSale&gt;()\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 .Where(h =&gt; h.DateOfSale &gt;= startDate)\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 .Where(h =&gt; h.DateOfSale &lt;= endDate)\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 .List();\r\n\u00a0\u00a0\u00a0 }\r\n\u00a0\r\n\u00a0\u00a0\u00a0 private IList&lt;MonthlyHatSales&gt; CalculateAverageHatWidthsPerMonth(IList&lt;HatSale&gt; hatSales)\r\n\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 return hatSales\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 .GroupBy(sale =&gt; sale.DateOfSale.Month)\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 .Select(group =&gt; new MonthlyHatSales\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 AverageHatWidth = group.Average(sale =&gt; sale.Hat.Width),\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 Month = group.Key\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 })\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 .ToList();\r\n\u00a0\u00a0\u00a0 }\r\n\u00a0\r\n\u00a0\u00a0\u00a0 public class MonthlyHatSales\r\n\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 public int Month { get; set;}\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 public double AverageHatWidth { get; set; }\r\n\u00a0\u00a0\u00a0 }\r\n}\r\n<\/pre>\n<p>Although this code reads much better, it&#8217;s not perfect by any means. For instance, it doesn&#8217;t obey the <a href=\"https:\/\/en.wikipedia.org\/wiki\/Single_responsibility_principle\">Single Responsibility Principle<\/a>: it deals with everything from parsing HTTP parameters to querying the database. We&#8217;ve already pulled this logic into separate methods, so a next step might be to try and pull these methods into separate classes. Regardless of what change you&#8217;re making, you should be extremely cautious about changing any code without any tests around it.<\/p>\n<h2>Conclusion<\/h2>\n<p>I&#8217;ve talked about how to write maintainable code, but is it the case that we always want maintainable code? Are there not occasions where we need to write a quick piece of code that we&#8217;re going to throw away immediately? This logic has one major flaw: our inability to predict the future. What begins as throw-away code can quickly become a vital piece of infrastructure, needing maintenance and extension for years. Even if the code is to be thrown away, the benefits of writing maintainable code can pay off much quicker than you might think. Often, a quick piece of code isn&#8217;t so quick after all, and you can easily find yourself surrounded by a baffling labyrinth of code in under a day.<\/p>\n<p>Finally, it&#8217;s important to have a sense of craftmanship. Through continuous practice we improve the quality of what we write, and form the habit of always writing clean, readable code.<\/p>\n<\/div>\n","protected":false},"excerpt":{"rendered":"<p>Writing maintainable code is hard. It must be understandable, testable and readable. Any one of these can be tricky, and together they seem pretty daunting. Thankfully, Michael Williamson makes it look easy to become a code craftsman.&hellip;<\/p>\n","protected":false},"author":221713,"featured_media":0,"comment_status":"open","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":{"_acf_changed":false,"footnotes":""},"categories":[143538],"tags":[4143,4229,4883,5440,5442,5441],"coauthors":[7200],"class_list":["post-1198","post","type-post","status-publish","format-standard","hentry","category-dotnet-development","tag-net","tag-net-framework","tag-net-home","tag-maintainable-code","tag-optimisation","tag-testable"],"acf":[],"_links":{"self":[{"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/posts\/1198","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/users\/221713"}],"replies":[{"embeddable":true,"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/comments?post=1198"}],"version-history":[{"count":5,"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/posts\/1198\/revisions"}],"predecessor-version":[{"id":80334,"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/posts\/1198\/revisions\/80334"}],"wp:attachment":[{"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/media?parent=1198"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/categories?post=1198"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/tags?post=1198"},{"taxonomy":"author","embeddable":true,"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/coauthors?post=1198"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}