## 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](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-i-xqz3evd) * [Part II](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-ii-o96s3wl4) * [Part III](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-iii-t7h3zkv) * [Part IV](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-iv-7sc3w8n) * [Part V](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-v-evj3zs9) * [Part VI](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-vi-cmj31om) * [Part VII](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-vii-8dk31x0) * [Part VIII](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-viii-8mn3352) * [Part IX](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-ix-7rr33ol) * [Part X](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-x-i7r34uj) * [Part XI](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-xi-sit35t1) * [Part XII](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-xii) * [Part XIII](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-xiii) * [Part XIV](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-xiv) * [Part XV](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-xv) * [Part XVI](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-xvi) * [Part XVII](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-xvii) * [Part XVIII](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-xviii) \ Let's continue... --- # Code Smell 91 - Test Asserts without Description *We are big fans of xUnit. But we don't care much for the programmers* \  > TL;DR: Use asserts with declarative descriptions. # Problems * Readability * Hard debugging * Time waste # Solutions 1. Put a nice descriptive assertion 2. Share guides for problem-solving # Sample Code ## Wrong ```php <? public function testNoNewStarsAppeared(): void { $expectedStars = $this->historicStarsOnFrame(); $observedStars = $this->starsFromObservation(); //These sentences get a very large collection $this->assertEquals($expectedStars, $observedStars); //If something fails we will have a very hard debugging time } ``` ## Right ```php <? public function testNoNewStarsAppeared(): void { $expectedStars = $this->historicStarsOnFrame(); $observedStars = $this->starsFromObservation(); //These sentences get a very large collection $newStars = array_diff($expectedStars, $observedStars); $this->assertEquals($expectedStars, $observedStars , 'There are new stars ' . print_r($newStars,true)); //Now we can see EXACTLY why the assertion failed with a clear and //Declarative Message } ``` # Detection Since *assert* and *assertDescription* are different functions, we can adjust our policies to favour the latter. # Tags * Test Smells # Conclusion Be respectful to the reader of your assertions. It might even be yourself! # More Info * [XUnit: Assert Description Deprecation](https://github.com/xunit/xunit/issues/350) # Credits Photo by [Startaê Team](https://unsplash.com/@startaeteam) on [Unsplash](https://unsplash.com/s/photos/dialogue) ___ > Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live. *John Woods* ___ # Code Smell 92 - Isolated Subclasses Names *If your classes are globals, use fully qualified names*  > TL;DR: Don't use abbreviations in subclasses # Problems * Readability * Mistakes # Solutions 1. Rename your classes to provide context 2. Use modules, namespaces or fully qualified names # Sample Code ## Wrong ```java abstract class PerserveranceDirection { } class North extends PerserveranceDirection {} class East extends PerserveranceDirection {} class West extends PerserveranceDirection {} class South extends PerserveranceDirection {} //Subclasses have short names and meaningless outside the hierarchy //If we reference East we might mistake it for the Cardinal Point ``` ## Right ```java abstract class PerserveranceDirection { } class PerserveranceDirectionNorth extends PerserveranceDirection {} class PerserveranceDirectionEast extends PerserveranceDirection {} class PerserveranceDirectionWest extends PerserveranceDirection {} class PerserveranceDirectionSouth extends PerserveranceDirection {} //Subclasses have fully quallified names ``` # Detection Automatic detection is not an easy task. We could enforce local naming policies for subclasses. # Tags * Naming # Conclusion Choose your names wisely. If your language supports it, use modules, namespaces and local scopes. # Relations [Code Smell 11 - Subclassification for Code Reuse](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-iii-t7h3zkv) # More Info * [What is in a name?](https://hackernoon.com/what-exactly-is-a-name-the-quest-part-i-fmw3udc) * [MAPPER](https://hackernoon.com/the-one-and-only-software-design-principle-1x983ylp) # Credits Photo by [Edvard Alexander Rølvaag](https://unsplash.com/@edvardr) on [Unsplash](https://unsplash.com/s/photos/hierarchy) ___ > The programmer's primary weapon in the never-ending battle against slow system is to change the intramodular structure. Our first response should be to reorganize the modules' data structures. *Frederick P. Brooks* ___ # Code Smell 93 - Send me Anything *Magic functions that can receive a lot of different (and not polymorphic arguments)* \  \ > TL;DR: Create a clear contract. Expect just one protocol. # Problems * Fail Fast principle violation * Error prune * Readability * [If polluting](https://hackernoon.com/how-to-get-rid-of-annoying-ifs-forever-zuh3zlo) * [Nulls](https://hackernoon.com/null-the-billion-dollar-mistake-8t5z32d6) * Bad Cohesion # Solutions 1. Take just one "kind" of input 2. Arguments should adhere to a single protocol. # Sample Code ## Wrong ```php <? function parseArguments($arguments) { $arguments = $arguments ?: null; //Always the billion-dollar mistake if (is_empty($arguments)) { $this->arguments = http_build_query($_REQUEST); //Global coupling and side effects } elseif (is_array($arguments)) { $this->arguments = http_build_query($arguments); } elseif (!$arguments) { //null unmasked $this->arguments = null; } else { $this->arguments = (string)$arguments; } } ``` ## Right ```php <? function parseArguments(array $arguments) { $this->arguments = $arguments; //much cleaner, isn't it ? } ``` # Detection We can detect this kind of methods when they do different things, asking for the argument *kind* # Tags * If Polluter # Conclusion Magic castings and flexibility have a price. They put the rubbish under the rug and violate fail fast principle. # Relations [Code Smell 69 - Big Bang (JavaScript Ridiculous Castings)](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-xiv) [Code Smell 06 - Too Clever Programmer](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-ii-o96s3wl4) [Code Smell 12 - Null](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-iii-t7h3zkv) # Credits Photo by [Hennie Stander](https://unsplash.com/@henniestander) on [Unsplash](https://unsplash.com/s/photos/juggler) ___ > Referential transparency is a very desirable property: it implies that functions consistently yield the same results given the same input, irrespective of where and when they are invoked. *Edward Garson* ___ # Code Smell 94 - Too Many imports *If your class relies on too many others, it will be coupled and fragile. A long import list is a good indicator.* \  > TL;DR: Don't import too much. # Problems * Coupling * Single Responsibility Principle violation * Low Cohesion # Solutions 1. Break the class 2. Hide intermediate accidental implementation # Sample Code ## Wrong ```java import java.util.LinkedList; import java.persistence; import java.util.ConcurrentModificationException; import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.ListIterator; import java.util.NoSuchElementException import java.util.Queue; import org.fermi.common.util.ClassUtil; import org.fermi.Data; //We rely on too many libraries public class Demo { public static void main(String[] args) { } } ``` ## Right ```java import org.fermi.domainModel; import org.fermi.workflow; //We rely on few libraries //and we hide their implementation //So maybe transitive imports are the same //but we don't break encapsulation public class Demo { public static void main(String[] args) { } } ``` # Detection We can set a warning threshold on our linters. # Tags * Coupling * Ripple Effect # Conclusion We need to think about dependencies when building our solutions to minimize Ripple Effect. # Relations [Code Smell 61 - Coupling to Classes](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-xiii) # More Info * [Coupling: The one and only software design problem](https://hackernoon.com/coupling-the-one-and-only-software-designing-problem-9z5a321h) * [Namespaces on Wikipedia](https://en.wikipedia.org/wiki/Namespace) # Credits Photo by [Zdeněk Macháček](https://unsplash.com/@zmachacek) on [Unsplash](https://unsplash.com/s/photos/pile) [Twitter](https://twitter.com/1447623706767921153) ___ > Fools ignore complexity. Pragmatists suffer it. Some can avoid it. Geniuses remove it. *Alan Perlis* ___ # Code Smell 95 - Premature Classification *We are over generalizers. We shouldn't create abstractions until we see enough concretions.* \  > TL;DR: Don't guess what the future will bring you. # Context Aristotelian Classification is a big problem in computer science. We tend to classify and name things **before** gathering enough knowledge and context. # Problems * Futurology * Bad designs # Solutions 1. Wait for concretions 2. Refactor late # Sample Code ## Wrong ```java class Rectangle { int length; int breadth; int area() { return length * breadth; } } //We are creating a premature abstraction //And misusing is-a relation since a Square "is a" Rectangle class Square extends Rectangle { int length; int area() { return length * length; } } ``` ## Right ```java class Rectangle { int length; int breadth; int area() { return length * breadth; } } class Square { int length; int area() { return length * length; } } //Square might-be a Rectangle //But it does not follow behaves-like relation so we won't go ahead //and create a strong relation between them //Maybe they are shapes. We don't have enough examples and protocol yet //We will not guess until further knowledge ``` # Detection An abstract class with just one subclass is an indicator of premature classification # Tags * Bad Design * Classification # Conclusion When working with classes, we name abstractions as soon as they *appear*. Our rule is to choose [good names](https://hackernoon.com/what-exactly-is-a-name-the-quest-part-i-fmw3udc) after the behaviour. We should not name our abstractions until we name our concrete subclasses. # Relations [Code Smell 11 - Subclassification for Code Reuse](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-iii-t7h3zkv) # More Info * [What is in a name](https://hackernoon.com/what-exactly-is-a-name-the-quest-part-i-fmw3udc) # Credits Photo by [Faye Cornish](https://unsplash.com/@fcornish) on [Unsplash](https://unsplash.com/s/photos/tree) ___ > Let us change our traditional attitude to the construction of programs: Instead of imagining that our main task is to instruct a computer what to do, let us concentrate rather on explaining to human beings what we want a computer to do. *Donald E. Knuth* [Software Engineering Great Quotes](https://hackernoon.com/400-thought-provoking-software-engineering-quotes) ___ And that’s all for now… The next article will explain 5 more code smells! \