Code smells are a classic. It smells because there are likely many instances where it could be edited or improved. Most of these smells are just hints of something that might be wrong. They are not required fixed per se… (You should look into it though.) Previous Code Smells Part I Part II Part III Part IV Part V Part VI Part VII Part VIII Part IX Part X Part XI Part XII Part XIII Part XIV Part XV Part XVI Part XVII Part XVIII Part XIX Part XX Part XXI Let's continue... Code Smell 106 - Production Dependent Code Don't add IFs checking for the production environment. TL;DR: Avoid adding conditionals related to production Problems Fail fast principle violation Lack of testability Solutions If completely necessary, model environments and test of them. ALL Context Sometimes, We need to create different behaviors in development and production. For example the strength of the passwords. In this case, we need to configure the environment with the strength strategy and test the strategy and not the environment itself. Sample Code Wrong def send_welcome_email(email_address, environment): if ENVIRONMENT_NAME == "production": print(f"Sending welcome email to {email_address} from Bob Builder <bob@builder.com>") else: print("Emails are sent only on production") send_welcome_email("john@doe.com", "development") # Emails are sent only on production send_welcome_email("john@doe.com", "production") # Sending welcome email to john@doe.com from Bob Builder <bob@builder.com> Right class ProductionEnvironment: FROM_EMAIL = "Bob Builder <bob@builder.com>" class DevelopmentEnvironment: FROM_EMAIL = "Bob Builder Development <bob@builder.com>" # We can unit test environments # and even implement different sending mechanisms def send_welcome_email(email_address, environment): print(f"Sending welcome email to {email_address} from {environment.FROM_EMAIL}") # We can delegate into a fake sender (and possible logger) # and unit test it send_welcome_email("john@doe.com", DevelopmentEnvironment()) # Sending welcome email to john@doe.com from Bob Builder Development <bob@builder.com> send_welcome_email("john@doe.com", ProductionEnvironment()) # Sending welcome email to john@doe.com from Bob Builder <bob@builder.com> Detection Manual [x] This is a design smell. We need to create empty development/production configurations and delegate them with customizable polymorphic objects. Tags Coupling Conclusion Avoid adding untestable conditionals. Create configurations delegating business rules. Use abstractions, protocol, and interfaces, avoid hard hierarchies. Relations Code Smell 56 - Preprocessors More Info How to avoid annoying ifs Credits Photo by on Birmingham Museums Trust Unsplash This tweet was inspired by @ Jan Giacomelli Twitter Complexity is a sign of technical immaturity. Simplicity of use is the real sign of a well design product whether it is an ATM or a Patriot missile. Daniel T. Ling Software Engineering Great Quotes Code Smell 107 - Variables Reuse Reusing variables makes scopes and boundaries harder to follow TL;DR: Don't read and write the same variable for different purposes Problems Readability Hidden problems Solutions Don't reuse variables to isolate scopes Extract Method Context When programming a script it is common to reuse variables. This leads to confusion and makes debugging harder. We should narrow the scope as much as possible. Sample Code Wrong // print line total double total = item.getPrice() * item.getQuantity(); System.out.println("Line total: " + total ); // print amount total total = order.getTotal() - order.getDiscount(); System.out.println( "Amount due: " + total ); // variable is reused Right function printLineTotal() { double total = item.getPrice() * item.getQuantity(); System.out.println("Line total: " + total ); } function printAmountTotal() { double total = order.getTotal() - order.getDiscount(); System.out.println( "Amount due: " + total ); } Detection Automatic [ ] Linters can use the parse tree to find variable definition and usages. Tags Readability Conclusion Avoid reusing variable names. Use more specific and different names. Relations Code Smell 03 - Functions Are Too Long More Info Refactoring 002 - Extract Method Credits Photo by on Sigmund Unsplash Simplicity before generality, use before reuse. Kevlin Henney Software Engineering Great Quotes Code Smell 108 - Float Assertions Asserting two float numbers are the same is a very difficult problem TL;DR: Don't compare floats Problems Wrong test results Fragile tests Fail fast principle violation Solutions Avoid floats unless you have REAL performance concerns Use arbitrary precision numbers If you need to compare floats compare with tolerance. Context Comparing float numbers is an old computer science problem. The usual solution is to use threshold comparisons. We recommend avoiding floats at all and trying to use infinite precision numbers. Sample Code Wrong Assert.assertEquals(0.0012f, 0.0012f); // Deprecated Assert.assertTrue(0.0012f == 0.0012f); // Not JUnit - Smell Right Assert.assertEquals(0.0012f, 0.0014f, 0.0002); // true Assert.assertEquals(0.0012f, 0.0014f, 0.0001); // false // last parameter is the delta threshold Assert.assertEquals(12 / 10000, 12 / 10000); // true Assert.assertEquals(12 / 10000, 14 / 10000); // false Detection Automatic [ ] We can add a check con on our testing frameworks to avoid checking for floats. assertEquals() Tags Test Smells Conclusion We should always avoid comparing floats. Relations Code Smell 71 - Magic Floats Disguised as Decimals More Info Fail fast Credits Photo by on Mika Baumeister Unsplash God made the natural numbers; all else is the work of man. Leopold Kronecker Software Engineering Great Quotes Code Smell 109 - Automatic Properties What happens if you combine 4 code smells? TL;DR: Avoid Getters, Avoid Setters, Avoid Metaprogramming. Think about Behavior. Problems Information Hiding Violation Mutability violation Fail Fast Principle Duplicate code when setting properties Solutions Remove automatic setters and getters Context Setters and getters are a bad industry practice. Many IDEs favor this code smell. Some languages provide explicit support to build anemic models and DTOs. Sample Code Wrong class Person { public string name { get; set; } } Right class Person { private string name public Person(string personName) { name = personName; //imutable //no getters, no setters } //... more protocol, probably accessing private variable name } Detection Automatic [ ] This is a language feature. We should avoid immature languages or forbid their worst practices. Tags Encapsulation Conclusion We need to think carefully before exposing our properties. The first step is to stop thinking about properties and focus solely on behavior. Relations Code Smell 28 - Setters Code Smell 68 - Getters Code Smell 70 - Anemic Model Generators Code Smell 40 - DTOs Code Smell 01 - Anemic Models More Info W3 schools Laziness I - Metaprogramming Laziness II - Code Wizards Refactoring 001 - Remove Setters The Evil Power of Mutants Fail Fast Credits Photo by on Kony Unsplash Nothing is harder than working under a tight deadline and still taking the time to clean up as you go. Kent Beck Software Engineering Great Quotes Code Smell 110 - Switches With Defaults Default means 'everything we don't know yet'. We cannot foresee the future. TL;DR: Don't add a default clause to your cases. Change it for an exception. Be Explicit. Problems Coupling Fail Fast principle violation Open closed principle violation Solutions Replace if and cases with polymorphism Change Default code to an Exception Context When using cases, we usually add a default case so it doesn't fail. Failing is always better than taking decisions without evidence. Since are also an smell, we can avoid them. case and switches Sample Code Wrong switch (value) { case value1: // if value1 matches the following will be executed.. doSomething(); break; case value2: // if value2 matches the following will be executed.. doSomethingElse(); break; default: // if value does not presently match the above values // or future values // the following will be executed doSomethingSpecial(); break; } Right switch (value) { case value1: // if value1 matches the following will be executed.. doSomething(); break; case value2: // if value2 matches the following will be executed.. doSomethingElse(); break; case value3: case value4: // We currently know these options exist doSomethingSpecial(); break; default: // if value does not match the above values we need to take a decision throw new Exception('Unexpected case ' + value + ' we need to consider it'); break; } Detection Semi Automatic [x] We can tell our linters to warn us on default uses unless there's an exception. Tags Fail Fast Conclusion Writing robust code doesn't mean we need to take decisions without evidence. Relations Code Smell 36 - Switch/case/elseif/else/if statements More Info Fail Fast Credits Photo by on Joshua Woroniecki Unsplash The cost of adding a feature isn’t just the time it takes to code it. The cost also includes the addition of an obstacle to future expansion. The trick is to pick the features that don’t fight each other. John Carmack Software Engineering Great Quotes And that’s all for now… The next article will explain 5 more code smells!