Infinite code smells!

\
We see several symptoms and situations that make us doubt the quality of our development.

Let's look at some possible solutions.

\
Most of these smells are just hints of something that might be wrong. They are not rigid rules.

## Previous Parts

[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)

\
Let's continue...

# Code Smell 66 - Shotgun Surgery

*Say it only once*

# Problems

* Bad Responsibilities Assignments
* Code Duplication
* Maintainability
* [Single Responsibility](https://en.wikipedia.org/wiki/Single-responsibility_principle) Violation.
* Copy-pasted code.

# Solutions


1. Refactor

# Sample Code

## Wrong

```php
final class SocialNetwork {

    function postStatus(string $newStatus) {
        if (!$user->isLogged()) {
            throw new Exception('User is not logged');
        }
        ///...
    }

    function uploadProfilePicture(Picture $newPicture) {
        if (!$user->isLogged()) {
            throw new Exception('User is not logged');
        }
        ///...
    }

    function sendMessage(User $recipient, Message $messageSend) {
        if (!$user->isLogged()) {
            throw new Exception('User is not logged');
        }
        ///...
    }
}
```

## Right

```php
<?

final class SocialNetwork {

    function postStatus(string $newStatus) {
        $this->assertUserIsLogged();
        ///...
    }

    function uploadProfilePicture(Picture $newPicture) {
        $this->assertUserIsLogged();
        ///...
    }

    function sendMessage(User $recipient, Message $messageSend) {
        $this->assertUserIsLogged();
        ///...
    }

    function assertUserIsLogged() {
        if (!$this->user->isLogged()) {
            throw new Exception('User is not logged');
            //This is just a simplification to show the code smell
            //Operations should be defined as objects with preconditions etc.
        }
    }
}
```

# Detection

Some modern linters can detect repeated patterns (not just repeated code) and also while performing our code reviews we can easily detect this problem and ask for a refactor.

# Tags

* Code Duplication

# Conclusion

Adding a new feature should be straightforward it our model maps 1:1 to real world and our responsibilities are in the correct places.
We should be alert for small changes spanning in several classes.

# More info

[Wikipedia](https://en.wikipedia.org/wiki/Shotgun_surgery)

[Refactoring Guru](https://refactoring.guru/es/smells/shotgun-surgery)

[NDpend Blog](https://blog.ndepend.com/shotgun-surgery)

[Dzone](https://dzone.com/articles/code-smell-shot-surgery)

# Credits

Photo by __[William Isted](https://unsplash.com/@williamisted)__ on __[Unsplash](https://unsplash.com/s/photos/shotgun)__


___

> Duplication is the primary enemy of a well-designed system.

*Robert Martin*


___

# Code Smell 67 - Middle Man

*Let's break Demeter's Law.*

\
 ![](https://cdn.hackernoon.com/images/RIiBoPtpMiRsMKX3dnzl5gb1Urj1-xnb3ht4.jpeg)

\
# Problems

* Unnecessary Indirection
* Empty Classes
* Readability

# Solutions


1. Remove Middle man.

# Sample Code

## Wrong

```java
public class Client {
    Address address;
    public ZipCode zipCode() {
        return address.zipCode();
    }
}

public class Address {
    private ZipCode zipCode;
    
    public ZipCode zipCode() {
        return new ZipCode('CA90210');
    }
}

public class Application {   
   ZipCode zipCode = client.zipCode();
}
```

## Right

```java
public class Client {
    Address address;
    //client now has to expose its address
    public address() {
        return address;
    }
}

public class Address {
    private ZipCode zipCode;
    
    public ZipCode zipCode() {
        return new ZipCode('CA90210');
    }
}

public class Application {   
   ZipCode zipCode = client.address().zipCode();
}
```

# Detection

Same as its [opposite smell](https://maximilianocontieri.com/code-smell-08-long-chains-of-collaborations), We can detect this small using parsing trees.

# Tags

* Coupling
* Declarative
* Readability

# Conclusion

This is exactly the opposite to [Message Chain](https://maximilianocontieri.com/code-smell-08-long-chains-of-collaborations). We make explicit the message chain.

# Relations

[Code Smell 08 - Long Chains Of Collaborations](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-ii-o96s3wl4)

[Code Smell 114 - Empty Class](https://maximilianocontieri.com/code-smell-114-empty-class)

# More info

[Refactoring Guru](https://refactoring.guru/smells/middle-man)

[Refactoring.com](https://refactoring.com/catalog/removeMiddleMan.html)

[C2 Wiki](https://wiki.c2.com/?MiddleMan)

[JetBrains](https://www.jetbrains.com/help/idea/remove-middleman.html#remove_middleman_example)

[Wikipedia](https://en.wikipedia.org/wiki/Law_of_Demeter)

## Credits

Photo by __[Dan Counsell](https://unsplash.com/@dancounsell)__ on __[Unsplash](https://unsplash.com/s/photos/robber)__


___

> Whenever I have to think to understand what the code is doing, I ask myself if I can refactor the code to make that understanding more immediately apparent.

*Martin Fowler*


___

# Code Smell 68 - Getters

*Getting things is widespread and safe. But it is a very bad practice.*

\
 ![](https://cdn.hackernoon.com/images/RIiBoPtpMiRsMKX3dnzl5gb1Urj1-6se3he8.jpeg)

# Problems

* Naming
* Information Hiding
* Coupling
* Encapsulation Violation
* Mutability
* Anemic Models

# Solutions


1. Avoid Getters
2. Use domain names instead
3. Protect your implementation decisions.

# Sample Code

## Wrong

```php
<?php

final class Window {
    public $width;
    public $height;
    public $children;

    public function getWidth() {
        return $this->width;
    }

    public function getArea() {
        return $this->width * $this->height;
    }

    public function getChildren() {
        return $this->children;
    }
}
```

## Right

```php
<?php

final class Window {
    private $width;
    private $height;
    private $children;

    public function width() {
        return $this->width;
    }

    public function area() {
        return $this->height * $this->width;
    }

    public function addChildren($aChild) {
        //do not expose internal attributes
        return $this->children[] = $aChild;
    }
}
```

# Detection

Getters coincide in certain scenarios with a true responsibility. It will be reasonable for a window to return its color and it may accidentally store it as color. so a *color()* method returning the attribute color might be a good solution.

*getColor()* breaks [bijection](https://maximilianocontieri.com/the-one-and-only-software-design-principle) since it is implementative and has no real counterpart on our [mappers](https://maximilianocontieri.com/what-is-wrong-with-software).

Most linters can warn us if they detect anemic models with getters and setters.

# Tags

* Information Hiding

# Conclusion

Getters and Setters are a bad established practice. Instead of focusing on object behavior (essential), we are desperate to know object guts (accidental) and violate their implementation.

# Relations

[Code Smell 28 - Setters](https://maximilianocontieri.com/code-smell-28-setters)

[Code Smell 01 - Anemic Models](https://maximilianocontieri.com/code-smell-01-anemic-models)

[Code Smell 64 - Inappropriate Intimacy](https://hackernoon.com/nude-models-part-ii-getters-sjo3ua2)

# More info

[Nude Models - Part II Getters](https://hackernoon.com/nude-models-part-ii-getters-sjo3ua2)

# Credits

Photo by [Vidar Nordli-Mathisen](https://unsplash.com/@vidarnm) on [Unsplash](https://unsplash.com/s/photos/pull)


___

> The value of a prototype is in the education it gives you, not in the code itself.

*Alan Cooper*


___

# Code Smell 69 - Big Bang (JavaScript Ridiculous Castings)

*This handy operator is a trouble maker.*

 ![](https://cdn.hackernoon.com/images/RIiBoPtpMiRsMKX3dnzl5gb1Urj1-swf3hwn.jpeg)

\
> TL;DR: Don't mix booleans with non-booleans.

# Problems

* Not Declarative Code
* Hard to debug
* Magic Castings
* Accidental Complexity

# Solutions


1. Be Explicit
2. Don't mix Booleans with non-booleans.
3. [Fail Fast](https://maximilianocontieri.com/fail-fast)
4. Be Smarter than your compiler.
5. Stay loyal to the bijection.

[The One and Only Software Design Principle](https://hackernoon.com/the-one-and-only-software-design-principle-1x983ylp)

# Sample Code

## Wrong

```javascript
!true // returns false
!false // returns true

isActive = true 
!isActive // returns false

age = 54
!age // returns false
array = []
!array // returns false
obj = new Object;
!obj // returns false

!!true // returns true
!!false // returns false

!!isActive // returns true
!!age // returns true
!!array // returns true
!!obj // returns true
```

## Right

```javascript
!true // returns false
!false // returns true

isActive = true 
!isActive // returns false

age = 54
!age // should return type mismatch (or 54 factorial!)
array = []
!array // should return type mismatch
obj = new Object;
!obj // should return type mismatch (what is an obejct negated in a real domain?)

!!true // returns true - it is idempotent
!!false // returns false - it is idempotent

!!isActive // returns true - it is idempotent
!!age // nonsense
!!array // nonsense
!!obj // nonsense
```

# Detection

Since this is a "feature" in some languages it would be hard to test. We can set programming policies or choose more [strict languages](https://dev.to/tmaximini/typescript-bang-operator-considered-harmful-3hhi).

We should detect *!* *!!* usages in non-boolean objects and warn our programmers.

# Tags

* Casting
* Coercion
* Javascript

# Conclusion

Languages like JavaScript divide their whole universe into *true* or *false* values. This decision hides errors when dealing with non booleans.

We should be very strict and keep booleans (and their behavior), far away from non booleans.

# Relations

[Code Smell 24 - Boolean Coercions](https://maximilianocontieri.com/code-smell-24-boolean-coercions)

[Code Smell 06 - Too Clever Programmer](https://hackernoon.com/how-to-find-the-stinky-parts-of-your-code-part-ii-o96s3wl4)

# More info

[Reddit](https://www.reddit.com/r/ProgrammerHumor/comments/6erd7r/the_best_thing_about_a_boolean_is_that_even_if/)

[Bennadel.com](https://www.bennadel.com/blog/3858-the-double-bang-operator-and-a-misunderstanding-of-how-javascript-handles-truthy-falsy-values.htm)

[Mozilla](https://developer.mozilla.org/en-US/docs/Glossary/Truthy)

# Credits

Photo by __[Greg Rakozy](https://unsplash.com/@grakozy)__ on __[Unsplash](https://unsplash.com/s/photos/universe)__


___

> It is easier to write an incorrect program than understand a correct one.

*Alan J Perlis*


___

# Code Smell 70 - Anemic Model Generators

 ![](https://cdn.hackernoon.com/images/RIiBoPtpMiRsMKX3dnzl5gb1Urj1-wjg3h5k.jpeg)

\
> TL;DR: Do not create anemic objects. Much less with automatic tools.

# Problems

* Anemic Objects
* Coupling to Implementation
* Harder to Debug

# Solutions


1. Create your objects manually.
2. Focus on essential behavior instead of accidental data storage.

# Sample Code

## Wrong

```php
AnemicClassCreator::create(
    'Employee',
    [
        new AutoGeneratedField('id', '$validators->getIntegerValidator()'),
        new AutoGeneratedField('name', '$validators->getStringValidator()'),
        new AutoGeneratedField('currentlyWorking', '$validators->getBooleanValidator()')
    ]);

class Employee extends AutoGeneratedObjectEmployee {
}

//We have magic setters and getters
//getId() , setId(), getName()
//Validation is not implicit
//Class are loaded via an autoloader

$john = new Employee;
$john->setId(1);
$john->setName('John');
$john->setCurrentlyWorking(true);

$john->getName(); //returns 'John'
```

## Right

```php
final class Employee {
    private $name;
    private $workingStatus;

    public function __construct(string $name, WorkingStatus $workingStatus) {
        //..
    }

    public function name(): string {
        return $this->name;
        //This is not a getter. It is Employee's responsibility to tell us her/his name
    }
}

//We have no magic setters or getters
//all methods are real and can be debugged
//Validations are implicit

$john = new Employee('John', new HiredWorkingStatus());

$john->name(); //returns 'John'
```

# Detection

Often, anemic models are generated with [meta-programming](https://maximilianocontieri.com/laziness-i-meta-programming).

We need to track these magic [code generators](https://maximilianocontieri.com/lazyness-ii-code-wizards).

# Tags

* Anemic

# Conclusion

Code Wizards, Meta-programming, and anemic models are all code smells.

We need to avoid these dark techniques.

Having to write *explicitly* the code makes us reflect on every piece of data we encapsulate.

# Relations

[Code Smell 01 - Anemic Models](https://maximilianocontieri.com/code-smell-01-anemic-models)

# More info

[Lazyness I - Metaprogramming](https://hackernoon.com/laziness-chapter-i-meta-programming-6s4l300e)

[Lazyness II - Code Wizards](https://hackernoon.com/lazyness-chapter-ii-code-wizards-3i9x3xtr)

# Credits

Photo by __[Lenny Kuhne](https://unsplash.com/@lennykuhne)__ on __[Unsplash](https://unsplash.com/s/photos/factory)__


___

> The best smells are something that's easy to spot and most of time lead you to really interesting problems. Data classes (classes with all data and no behavior) are good examples of this. You look at them and ask yourself what behavior should be in this class.

*Martin Fowler*


___

And that’s all for now…

Next article will explain 5 more code smells!

\


Maker

Chain

Mozilla

Winner of Noonies 2020 for the award Hacker Noon Contributor of the Year - REFACTORING.

How to Find The Stinky Parts of Your Code [Part XIV]

Too Long; Didn't Read

People Mentioned

Companies Mentioned

Coins Mentioned

@mcsee

RELATED STORIES