{"id":570,"date":"2009-04-27T00:00:00","date_gmt":"2009-04-27T00:00:00","guid":{"rendered":"https:\/\/test.simple-talk.com\/uncategorized\/exploring-smelly-code\/"},"modified":"2021-05-17T18:36:42","modified_gmt":"2021-05-17T18:36:42","slug":"exploring-smelly-code","status":"publish","type":"post","link":"https:\/\/www.red-gate.com\/simple-talk\/development\/dotnet-development\/exploring-smelly-code\/","title":{"rendered":"Exploring Smelly Code"},"content":{"rendered":"<div id=\"pretty\">\n<h1>Exploring Smelly Code<\/h1>\n<h2>What is Smelly Code?<\/h2>\n<p>&#8220;Smelly Code&#8221; is code in need of refactoring.\u00a0 Kent Beck quotes his grandmother &#8220;If it smells bad, change it.&#8221;\u00a0 There are many ways that code can smell bad.\u00a0 Most of these have been categorized as code smells with associated refactors that can resolve the problems with the code that gave the foul smell.<\/p>\n<p>This provides another way to identify problems in code and associate specific problems with common solutions.<\/p>\n<h2>A Brief Survey of Smells<\/h2>\n<p>Some smells are easy to recognize and relatively easy to resolve.\u00a0 Other smells are much more subtle and can be difficult to resolve.\u00a0 Some smells are even controversial and not always accepted as even causing a code problem.<\/p>\n<p>It is rather easy to identify code that smells of &#8220;Long Method&#8221;.\u00a0 In fact, we have seen how metrics can be applied to automate pinpointing code with this smell.\u00a0 On the controversial side, can code smell of comments?\u00a0 Does having too many comments imply bad code?\u00a0 Sometimes yes!<\/p>\n<p>With meaningful variable names and meaningful method names, there should be no need to have comments explain what your code is doing.\u00a0 Comments should strictly help explain why your code is doing what it is doing.\u00a0 As you look through your code, note the sections that are heavily commented.\u00a0 Chances are, you can eliminate some of these comments by renaming methods or extracting code to create new methods.\u00a0 The original comment may even provide guidance for the new method&#8217;s name.<\/p>\n<p>In this article, we will review three of the most common smells found in application code.\u00a0 We will review what the smell looks like, why we care about the smell, and what we can do about it.<\/p>\n<h2>Shotgun Surgery<\/h2>\n<p>Shotgun Surgery pops up when you have to make changes throughout the code base to implement a single requirement.\u00a0 This may often be caused by &#8220;copy and paste&#8221; programming.\u00a0 This may also be caused by not properly leveraging inheritance or not recognizing when related classes could have a common base class.<\/p>\n<p>Whatever the cause, there is one common solution.\u00a0 Identify the code that has to be changed.\u00a0 Move this code to a common location, and change every reference to call the new centralized implementation.<\/p>\n<h3>What Does It Look Like<\/h3>\n<div class=\"indent\">\n<p>A common example might be the steps needed for object initialization.\u00a0 Let&#8217;s consider the not so humble connection object.\u00a0 Typically, there are several steps needed to initialize the connection object.\u00a0 These steps are also subject to periodic changes but will most likely be the same throughout the application.\u00a0 Unfortunately, this commonality is not often properly leveraged.<\/p>\n<p>A typical data access implementation may often include code similar to this:<\/p>\n<pre class=\"lang:c# theme:vs2012\">public SqlDataReader GetData()\r\n{\r\n\u00a0\u00a0\u00a0 SqlConnection con = new SqlConnection(ConnectionString);\r\n\u00a0\u00a0\u00a0 SqlCommand cmd = con.CreateCommand();\r\n\u00a0\u00a0\u00a0 con.Open();\r\n\u00a0\u00a0\u00a0 cmd.CommandType = CommandType.StoredProcedure;\r\n\u00a0\u00a0\u00a0 cmd.CommandText = \"spSampleStoredProc\";\r\n\u00a0\u00a0\u00a0 return cmd.ExecuteReader(CommandBehavior.CloseConnection);\r\n}\r\n\u00a0\r\n<\/pre>\n<p>The first four lines of code will be the same if every &#8220;GetData&#8221; type method.\u00a0 Shotgun surgery comes into play if we want to change this logic.\u00a0 Suppose you get a new requirement to explicitly set the ConnectionTimeout or the CommandTimeOut or there is a new best practice for initializing these objects.\u00a0 To implement such a new requirement, every &#8220;GetData&#8221; type method will have to be modified.<\/p>\n<p>This may seem like an obvious mistake that no one would ever make, but it does happen.\u00a0 Most of the time, this problem is much more subtle.\u00a0<\/p>\n<p>Consider a logging strategy implemented in every class performing logging operations, or any object with multiple steps to initialize.\u00a0 We will also see shotgun surgery rear its head in the examples we review a little later.<\/p>\n<\/div>\n<h3>Why Do We Care?<\/h3>\n<div class=\"indent\">\n<p>If you have ever had to update such code you know how time consuming it can be to make such changes.\u00a0 If you have never had to make such changes, consider yourself lucky.\u00a0<\/p>\n<p>If takes time to locate every place that needs to be changed. It takes extra testing time to ensure that every places was properly updated, and following such a pattern makes it that much more difficult to write the original code.<\/p>\n<\/div>\n<h3>What Do We Do About It?<\/h3>\n<div class=\"indent\">\n<p>A much better solution is to move the redundant code to a common function and that replace the multiple instances with references to the new function.\u00a0 This is the basis for the Factory pattern.<\/p>\n<p>The example shown earlier could be rewritten as:<\/p>\n<pre class=\"lang:c# theme:vs2012\">private SqlCommand PrepareCommand(string storedProcedureName)\r\n{\r\n\u00a0\u00a0\u00a0 SqlConnection con = new SqlConnection(ConnectionString);\r\n\u00a0\u00a0\u00a0 SqlCommand cmd = con.CreateCommand();\r\n\u00a0\u00a0\u00a0 con.Open();\r\n\u00a0\u00a0\u00a0 cmd.CommandType = CommandType.StoredProcedure;\r\n\u00a0\u00a0\u00a0 cmd.CommandText = storedProcedureName;\r\n\u00a0\u00a0\u00a0 return cmd;\r\n}\r\n\u00a0\r\npublic SqlDataReader GetData()\r\n{\r\n\u00a0\u00a0\u00a0 SqlCommand cmd = PrepareCommand(\"spSampleStoredProc\");\r\n\u00a0\u00a0\u00a0 return cmd.ExecuteReader(CommandBehavior.CloseConnection);\r\n}\r\n\u00a0\r\n<\/pre>\n<p>The common approach is to use the ExtractMethod refactor to create a new method with the shotgun code and then update every method that duplicated the shotgun code to call the new method<\/p>\n<\/div>\n<h2>Feature Envy<\/h2>\n<p>Feature envy starts smelling when methods in an object use the methods or properties of another object more than its own methods and properties.\u00a0\u00a0 We say that this method is envious of the features in the other object.\u00a0 This is not always a bad thing.\u00a0 Sometimes, the object is really acting as an adapter to a legacy object, or perhaps the object is implementing a decorator pattern.\u00a0<\/p>\n<p>These implementations do not imply the feature envy smell.\u00a0 Adapter and decorator are legitimate patterns to solve underlying design problems.\u00a0 Feature Envy is a smell that points to a new design flaw in its own right.<\/p>\n<h3>What Does It Look Like?<\/h3>\n<div class=\"indent\">\n<p>There are several examples of Feature Envy that are worth exploring.\u00a0 Consider a &#8220;Customer&#8221; class that exposes a mailing Address property.\u00a0 If the details for the address are stored in an Address object, then the implementation for the mailing address might be similar to this:<\/p>\n<pre class=\"lang:c# theme:vs2012\">private Address currentAddress = null;\r\npublic string MailingAddress()\r\n{\r\n\u00a0\u00a0\u00a0 StringBuilder sb = new StringBuilder ();\r\n\u00a0\u00a0\u00a0 sb.Append(currentAddress.AddressLine1);\r\n\u00a0\u00a0\u00a0 sb.Append(\"\\n\");\r\n\u00a0\u00a0\u00a0 sb.Append(currentAddress.City + \", \" + currentAddress.State);\r\n\u00a0\u00a0\u00a0 sb.Append(\"\\n\");\r\n\u00a0\u00a0\u00a0 sb.Append(currentAddress.PostalCode);\r\n\u00a0\u00a0\u00a0 return sb.ToString();\r\n}\r\n<\/pre>\n<p>Formatting the mailing address could and should be in the address class.\u00a0 Having this logic in the Customer class makes the main logic in the Customer class harder to follow.<\/p>\n<p>Other examples may include a ShoppingCart object that calculates the price for each Item in the cart instead of delegating that to the Item iteself.\u00a0<\/p>\n<p>Even more dangerous, instead of a well defined class like Customer being envious of the properties in Address, imagine multiple web pages being envious of the Address details.<\/p>\n<p>A simplistic way to identify potentially envious code is to track any methods in an object that make references to another object above a specified threshold.\u00a0 For example, if a single method in the Logger class accesses more than five properties in the Borrower object, then there is probably a method embedded there that is just waiting to be moved to the Borrower object.<\/p>\n<\/div>\n<h3>Why Do We Care?<\/h3>\n<div class=\"indent\">\n<p>You may look at this code above and not see a problem.\u00a0 You may even be able to write such code without any problems.\u00a0 However, eventually, such coding practices will create a maintenance nightmare.\u00a0 There are two main problems.<\/p>\n<p>Polluting an unsuspecting object with logic that better belongs in another object hides the true purpose of our unsuspecting object.\u00a0 In our earlier examples, the ShoppingCart&#8217;s main task is not to calculate the price for every item in the cart.\u00a0 This logic only detracts from the main purpose and violates the cohesion principle.\u00a0 Because our unsuspecting object includes functionality that would be better housed somewhere else, it is not very cohesive.\u00a0 When you are trying to maintain the code, you may have difficulty finding the logic since it is not where you would expect to find it.<\/p>\n<p>There is also the potential problem with duplicated code.\u00a0 If a particular piece of logic is not where it is supposed to be, there is a good chance that it might be repeated in several wrong places.\u00a0 If you ever need to change this functionality, not only will you have to search to find the code, you also have to ensure that you have found everywhere that needs to be updated.\u00a0\u00a0 Sound familiar?\u00a0 This is one way that Shotgun Surgery starts stinking up our code.<\/p>\n<\/div>\n<h3>What Do We Do About It?<\/h3>\n<div class=\"indent\">\n<p>We rely on the MoveMethod refactor and potential ExtractMethod as well to resolve these types of issues.\u00a0<\/p>\n<p>The address example above could be rewritten as:<\/p>\n<pre class=\"lang:c# theme:vs2012\">\u00a0\u00a0\u00a0 public\u00a0 class Address\r\n\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 public override string ToString()\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 StringBuilder sb = new StringBuilder();\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 sb.Append(AddressLine1);\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 sb.Append(\"\\n\");\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 sb.Append(City + \", \" + State);\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 sb.Append(\"\\n\");\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 sb.Append(PostalCode);\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 return sb.ToString();\r\n\u00a0\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0\u00a0 . . .\r\n\u00a0\u00a0\u00a0 }\r\n<\/pre>\n<p>And in the Customer class, the MailingAddress property can be reduced to:<\/p>\n<pre class=\"lang:c# theme:vs2012\">\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 private Address currentAddress = null;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 public string MailingAddress()\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 return currentAddress.ToString() ;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n<\/pre>\n<p>Not only does this make the Customer class more highly cohesive and add more responsibility to the Address class, we are also shielded from the details of how the address is stored and how to properly format it.\u00a0 The Address object can now handle the details of formatting the address properly for international addresses or any other future requirements without requiring Customer and any other objects that need to render an address to get bogged down in these details.<\/p>\n<\/div>\n<h2>Smelly Switches<\/h2>\n<p>Smelly switches refer to code using a switch statement to decide which of a series of potential actions to take.\u00a0 If the same switch statement is repeated in multiple places, at the very least we have duplicated code.\u00a0 At the very worse, we have a maintenance nightmare.\u00a0 Not all switch statements are problems, but they are all worth looking into to make sure that a better design is not waiting to come out.<\/p>\n<h3>What Does It Look Like?<\/h3>\n<div class=\"indent\">\n<p>When you look over your code base and find the same switch statement repeated, you may have code that smells of switch statements on your hands.\u00a0<\/p>\n<p>Code snippets like this hint that you have a switch smell in your code:<\/p>\n<pre class=\"lang:c# theme:vs2012\">\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 switch (state)\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 case \"SC\":\r\n\u00a0\u00a0\u00a0\u00a0\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\u00a0\u00a0\u00a0\u00a0 \/\/ Process Broker\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 break;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 case \"AL\":\r\n\u00a0\u00a0\u00a0\u00a0\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\u00a0\u00a0\u00a0\u00a0 \/\/ Process Retailer\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 break;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 case \"GA\":\r\n\u00a0\u00a0\u00a0\u00a0\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\u00a0\u00a0\u00a0\u00a0 \/\/ Process Supplier\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 break;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 case \"KY\":\r\n\u00a0\u00a0\u00a0\u00a0\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\u00a0\u00a0\u00a0\u00a0 \/\/ Process Transport\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 break;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n<\/pre>\n<p>\u00a0Now if you have such a switch statement only once, then it is probably not an issue, but if you have this same, switch statement in multiple places, then you might have more of a problem that needs to be addressed.\u00a0 If the individual case statements are likely to change, refactoring to eliminate the switch statement will definitely pay off in the long run.<\/p>\n<\/div>\n<h3>Why Do We Care<\/h3>\n<div class=\"indent\">\n<p>You may be asking yourself, &#8220;What is wrong with this&#8221;?\u00a0 Or you may even be thinking &#8220;I do this all the time!\u00a0 I have never had any problems with it!&#8221;\u00a0\u00a0 &#8220;A switch statement is so much simpler than a collection of if statements!&#8221;\u00a0 All of this may be true.<\/p>\n<p>\u00a0The problem is not comparing if statements with switch statements.\u00a0 The problem lies in the amount of work needed to unwind the business logic embedded in the complex conditionals.\u00a0<\/p>\n<p>Object oriented purists will point out all that has to change if you add a new state.\u00a0 Consider the problems that can arise with the state specific business logic spread throughout the system.\u00a0 When it is time to add a new state, you will have to search through the code base to find every switch statement, and update them individually.\u00a0 This sounds a little like shotgun surgery again only on a larger scale.<\/p>\n<p>If the switch statement has only a handful of case statements and the case statements are not likely to change, the complexity of the method will be manageable.\u00a0 But as the number of cases statements grow, the complexity can quickly get out of hand and the overhead of making the updates system wide can be not only a maintenance nightmare but also a deployment nightmare.<\/p>\n<\/div>\n<h3>What Do We Do About It<\/h3>\n<div class=\"indent\">\n<p>We can often eliminate these switch statements with an object hierarchy.<\/p>\n<p>For our state specific business logic example, we can build an object hierarchy implementing a strategy pattern.\u00a0 Every method needing state specific logic would have a corresponding method in the object hierarchy.\u00a0 Instead of duplicating the switch statement everywhere, we use a factory to return the appropriate object and then simply call the corresponding method from the returned object.<\/p>\n<p>Our object hierarchy might look similar to this:<\/p>\n<\/div>\n<p class=\"illustration\"><img loading=\"lazy\" decoding=\"async\" src=\"https:\/\/www.red-gate.com\/simple-talk\/wp-content\/uploads\/imported\/702-image002.jpg\" alt=\"702-image002.jpg\" width=\"624\" height=\"158\" \/><\/p>\n<div class=\"indent\">\n<p>The key thing is for all of our objects to have a common ancestor.\u00a0 The inheritance depth can be to whatever level makes sense for your application.\u00a0 If two states have a common implementation, let them share a common ancestor and put that common implementation in the ancestor class, but there is no need to create a more complicated structure than is necessary.\u00a0<\/p>\n<p>A method such as this:<\/p>\n<pre class=\"lang:c# theme:vs2012\">\u00a0\u00a0\u00a0 public double CalculateSalesTax\u00a0 (string state, double price)\r\n\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 switch (state)\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 case \"SC\":\r\n\u00a0\u00a0\u00a0\u00a0\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\u00a0\u00a0\u00a0\u00a0 return price * SCTAX ;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 case \"AL\":\r\n\u00a0\u00a0\u00a0\u00a0\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\u00a0\u00a0\u00a0\u00a0 return price * ALTAX ;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 case \"GA\":\r\n\u00a0\u00a0\u00a0\u00a0\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\u00a0\u00a0\u00a0\u00a0 return price * GATAX ;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 case \"KY\":\r\n\u00a0\u00a0\u00a0\u00a0\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\u00a0\u00a0\u00a0\u00a0return price * KYTAX;\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 }\r\n\u00a0\u00a0\u00a0 }\r\n\u00a0\r\n<\/pre>\n<p>Can be reduced to:<\/p>\n<pre class=\"lang:c# theme:vs2012\">\u00a0\u00a0\u00a0 public double CalculateSalesTax(string state, double price)\r\n\u00a0\u00a0\u00a0 {\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 StateBase activeState = Factory.CreateState(state);\r\n\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0\u00a0 return activeState.CalculateSalesTax(price);\r\n\u00a0\u00a0\u00a0 }\u00a0 \r\n\u00a0\r\n<\/pre>\n<p>This simple method can handle invoking the state specific logic regardless of how many states are supported.\u00a0 Each state object is easy to implement because it has to worry about how to take care of that one state.<\/p>\n<p>The individual state objects can also benefit from inheritance making their implementation even easier.\u00a0 As new states are added, we don&#8217;t have to search through the codebase for what needs to be changed.\u00a0 We need to simply implement the new state object.\u00a0\u00a0 Depending on how the factory is implemented; we may not have to change any existing code, simply deploy a new assembly with the new state object.<\/p>\n<\/div>\n<h2>Conclusion<\/h2>\n<p>Sometimes bad things happen to good code.\u00a0 Even the best code can start to smell if not carefully monitored.\u00a0 Fortunately, we can still intervene and save our code even after it starts to smell.<\/p>\n<p>You can get more information on the basic Refactors here: <a href=\"http:\/\/www.refactoring.com\/catalog\/index.html\">http:\/\/www.refactoring.com\/catalog\/index.html<\/a><\/p>\n<p>A good catalog for the code smells can be found here:\u00a0 <a href=\"http:\/\/mikamantyla.eu\/BadCodeSmellsTaxonomy.html\">http:\/\/www.soberit.hut.fi\/mmantyla\/BadCodeSmellsTaxonomy.htm<\/a><\/p>\n<\/div>\n","protected":false},"excerpt":{"rendered":"<p>Bad Code Smells are similar in concept to Development-level Antipatterns. They don&#8217;t describe bad programming aesthetics and you can&#8217;t sniff them out precisely with code metrics. They describe code in need of refactoring in rich language such as &#8216;Speculative Generality&#8217;, &#8216;Inappropriate Intimacy&#8217; or &#8216;shotgun surgery&#8217;. They&#8217;re useful because they give us words to describe antipatterns that we all come across in code. Nick Harrison explains&#8230;&hellip;<\/p>\n","protected":false},"author":221853,"featured_media":0,"comment_status":"open","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":{"_acf_changed":false,"footnotes":""},"categories":[143538],"tags":[4143,4229,4500],"coauthors":[11316],"class_list":["post-570","post","type-post","status-publish","format-standard","hentry","category-dotnet-development","tag-net","tag-net-framework","tag-refactoring"],"acf":[],"_links":{"self":[{"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/posts\/570","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\/221853"}],"replies":[{"embeddable":true,"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/comments?post=570"}],"version-history":[{"count":5,"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/posts\/570\/revisions"}],"predecessor-version":[{"id":72958,"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/posts\/570\/revisions\/72958"}],"wp:attachment":[{"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/media?parent=570"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/categories?post=570"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/tags?post=570"},{"taxonomy":"author","embeddable":true,"href":"https:\/\/www.red-gate.com\/simple-talk\/wp-json\/wp\/v2\/coauthors?post=570"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}