Image by cookie_studio on Freepik 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 Part XXII Part XXIII Part XXIV Let's continue... Code Smell 121 - String Validations You need to validate strings. So you don't need strings at all TL;DR: Search for missing domain objects when validating strings. Problems Primitive obsession. Bijection Fault Validated strings are a subset of all possible strings. principle violation. Fail Fast Single Responsibility Principle violation. DRY Principle Violation. Solutions Create a first-class object representing the concept under the MAPPER Context Serious software has lots of string validations. Often, They are not in the correct places. This leads to non-robust and corrupt software. The simple solution is to build only real-world and valid abstractions. Sample Code Wrong <?

// First Example: Address Validation
class Address { 
  function __construct(string $emailAddress) {
     // String validation on Address class violates SRP
     $this->validateEmail($emailAddress);
     // ...
   }
  
  private function validateEmail(string $emailAddress) {
    $regex = "/[a-zA-Z0-9_-.+]+@[a-zA-Z0-9-]+.[a-zA-Z]+/";
    // Regex is a sample / It might be wrong
    // Emails and Urls should be first class objects

    if (!preg_match($regex, $emailAddress))
    {
      throw new Exception('Invalid email address ' . emailAddress);
    }    
  }
}

// Second Example: Wordle

class Wordle { 
  function validateWord(string $wordleword) {
    // Wordle word should be a real world entity. Not a subset of strings
  }
 } Right <?

//First Example: Address Validation
class Address { 
  function __construct(EmailAddress $emailAddress) {
     // Email is always valid / Code is cleaner
     // ...
   }
}
  
class EmailAddress { 
  // We can reuse this object many times avoiding copy-pasting
  string $address; 
  private function __construct(string $emailAddress) {
    $regex = "/[a-zA-Z0-9_-.+]+@[a-zA-Z0-9-]+.[a-zA-Z]+/";
    // Regex is a sample / It might be wrong
    // Emails and Urls are first class objects

    if (!preg_match($regex, $emailAddress))
    {
      throw new Exception('Invalid email address ' . emailAddress);
    }   
    $this->address = $emailAddress;
  }
}

// Second Example: Wordle

class Wordle { 
  function validateWord(WordleWord $wordleword) {
    // Wordle word is a real world entity. Not a subset of string
  }
 }

class WordleWord { 
  function __construct(string $emailAddress) {
    // Avoid building invalid world words
    // For example length != 5
  }
 } Detection Semi-Automatic [x] We can check all constructors validating strings and the missing concepts. reify Tags Primitive Obsession Conclusion Small objects are hard to find. Primitive obsessors about this kind of indirections. always complain Creating these new small concepts keeps our model loyal to the bijection and ensures our models are always healthy. Relations Code Smell 41 - Regular Expression Abusers Code Smell 04 - String Abusers More Info The One and Only Software Design Principle How to Develop a Wordle Game Object Reification Credits Photo by on Brett Jordan Unsplash Less than 10% of the code has to do with the ostensible purpose of the system; the rest deals with input-output, data validation, data structure maintenance, and other housekeeping. Mary Shaw Software Engineering Great Quotes Code Smell 122 - Primitive Obsession Objects are there for the picking. Even the smallest ones. TL;DR: Use small objects instead of primitive ones. Problems Code Duplication Small Objects Missing principle violation. Fail Fast Bijection Fault Subset violations: Emails are a subset of strings, Valid Ages are a subset of Real, Ports are a subset of Integers, etc. We spread Logic and Behavior in many places. Premature Optimization. Solutions Create Small Objects Build missing abstractions using MAPPER Use Value-Objects Context We are very lazy to create small objects. We are also lazy to separate and What How. We like very much to understand the of how things work. internals We need to start thinking in a white box way and looking at the protocol and behavior of small components. Sample Code Wrong // Samples borrowed with permission from
// https://towardsdev.com/why-a-host-is-not-a-string-and-a-port-is-not-an-integer-595c182d817c

var port = 8080;

var in = open("example.org", port);
var uri = urifromPort("example.org", port);
var address = addressFromPort("example.org", port);
var path = pathFromPort("example.org", port); Right // Samples borrowed with permission from
// https://towardsdev.com/why-a-host-is-not-a-string-and-a-port-is-not-an-integer-595c182d817c

const server = Port.parse(this, "www.kivakit.org:8080");
// Port is a smallobject with responsibilities and protocol

let in = port.open(this);
const uri = port.asUri(this);
const address = port.asInetSocketAddress();
const path = port.path(this, "/index.html"); Detection Manual [x] We can automate checks on constructors for small objects missing opportunities. Tags Primitive Obsession Conclusion We need to transform our strings, numbers, and arrays into small objects. Relations Code Smell 121 - String Validations Code Smell 04 - String Abusers More Info Why a Host is not a String and a Port is not an Integer Primitive Obsession — A Code Smell that Hurts People the Most Refactoring Guru Credits Photo by on K. Mitch Hodge Unsplash Iteration allows us to progressively approach some goal. We can discard the steps that take us further away and prefer the steps that move us nearer. This is in essence how evolution works. It is also at the heart of how modern machine learning (ML) works. Dave Farley Software Engineering Great Quotes Code Smell 123 - Mixed 'What' and 'How' We love looking at the internal gears of the clock, but we need to start focusing on the hands. TL;DR: Don't mess with implementation details. Be declarative. Not imperative. Problems Accidental Coupling Coupling Lack of design for change distinguish the 'how' and the 'what'. Comments Solutions Separate 'What' and 'How' concerns. Context Separating concerns is very difficult in the software industry. Functional software survives ages. Implementation software brings coupling and is harder to change. Choosing wise declarative names is a daily challenge. Sample Code Wrong class Workflow {
    moveToNextTransition() {
        // We couple the business rule with the accidental implementation
        if (this.stepWork.hasPendingTasks) {
            throw new Exception('Preconditions are not met yet..');
        } else {
            this.moveToNextStep();
        }
    }
} Right class Workflow {
    moveToNextTransition() {
        if (!this.canWeMoveOn()) {
            throw new Exception('Preconditions are not met yet..');
        } else {
            this.moveToNextStep();
        }
    }

    canWeMoveOn() {
        // We hide accidental implementation 'the how'
        // under the 'what'
        return !this.stepWork.hasPendingTasks();
    }
} Detection Manual [x] This is a semantic and naming smell. Tags Readability Conclusion We need to choose good names and add indirection layers when necessary. Of course, will fight us, telling us we are wasting computational premature optimizators resources and they need to know the insights we are hiding from them. Relations Code Smell 92 - Isolated Subclasses Names Code Smell 05 - Comment Abusers More Info On premature optimizators Coupling What is in a name? Credits Photo by on Josh Redd Unsplash The idea of this smell is here: Code Smell 118 - Return False's comment and here: Twitter We are constantly interfacing with other people's code that might not live up to our high standards and dealing with inputs that may or may not be valid. So we are taught to code defensively. We use assertions to detect bad data and check for consistency. Andrew Hunt Software Engineering Great Quotes Code Smell 124 - Divergent Change You change something in a class. You change something unrelated in the same class TL;DR: Classes should have just one responsibility and one reason to change. Problems Coupling Code Duplication Low Cohesion Single Responsibility Principle violation Solutions Extract class Context We create classes to fulfill responsibilities. If an object does too much, it might change in different directions. Sample Code Wrong class Webpage {
  
  renderHTML() {
    renderDocType();
    renderTitle();
    renderRssHeader();
    renderRssTitle();
    renderRssDescription();
   // ...
  }
  // HTML render can change

  renderRssDescription() {
   // ...
  }

  renderRssTitle() {
   // ...
  }

  renderRssPubDate() {
   // ...
  }
  // RSS Format might change

} Right class Webpage {
  
  renderHTML() {
    this.renderDocType();
    this.renderTitle();
    (new RSSFeed()).render();
    this.renderRssTitle();
    this.renderRssDescription();
   // ...
  }
  // HTML render can change
}

class RSSFeed {
  render() {
    this.renderDescription();
    this.renderTitle();
    this.renderPubDate();
    // ...
  }  
  // RSS Format might change
  // Might have unitary tests
  // etc
} Detection Semi Automatic [x] We can automatically detect large classes or track changes. Tags Coupling Conclusion Classes must follow the Single Responsibility Principle and have just one reason to change. If they evolve in different ways, they are doing too much. Relations Code Smell 34 - Too Many Attributes Code Smell 94 - Too Many imports Code Smell 14 - God Objects More Info Refactoring.guru A design that doesn’t take change into account risks major redesign in the future. Erich Gamma Software Engineering Great Quotes Code Smell 125 - 'IS-A' Relationship We learned at school that inheritance represents an 'is-a' relationship. It is not. TL;DR: Think about protocol and behavior, forget inheritance Problems Bad models Unexpected behavior Subclass overrides Liskov substitution principle Violation Solutions Think in terms of behavior behaves-as-a Prefer composition over inheritance Subclassify always following the 'behaves-as-a' relation Context comes from the data world. IS-A relation We learned with structured design and data modeling. ERDs Now, we need to think in terms of behavior. Behavior is essential, data is . accidental Sample Code Wrong // If you made Square derive from Rectangle, 
// then a Square should be usable anywhere you expect a rectangle

#include <iostream>

Rectangle::Rectangle(const unsigned width, const unsigned height):
    m_width(width),
    m_height(height)
{
}

unsigned Rectangle::getWidth() const
{
    return m_width;
}

void Rectangle::setWidth(const unsigned width)
{
  /* Width and Height are independent */
    m_width = width;
}

unsigned Rectangle::getHeight() const
{
    return m_height;
}

void Rectangle::setHeight(const unsigned height)
{
    m_height = height;
}

unsigned Rectangle::area() const
{
  /*Valid for both Rectangles and Squares*/
    return m_height * m_width;
}

Square::Square(const unsigned size)
    : Rectangle(size, size)
{
}

// OK for squares, bad for rectangles
// Protocol is bad, width and height are not relevant on squares
void Square::setWidth(const unsigned size)
{
    m_height = size;
    m_width = size;
}

// OK for squares, bad for rectangles
// Protocol is bad, width and height are not relevant on squares
void Square::setHeight(const unsigned size)
{
    m_height = size;
    m_width = size;
}

void process(Rectangle& r)
{
    unsigned h = 10;
    auto w = r.getWidth();
    r.setHeight(h);

    std::cout << "Expected area: " << (w*h) << ", got " << r.area() << "\n";
    // area is not well defined in squares
    // every square IS-A rectangle, but does not behave-like a rectangle
}

int main()
{
    Rectangle rectangle{3,4};
    Square square{5};
    process(rectangle);
    process(square);
} Right // If you made Square derive from Rectangle, 
// then a Square should be usable anywhere you expect a rectangle

#include <iostream>

Rectangle::Rectangle(const unsigned width, const unsigned height):
    m_width(width),
    m_height(height)
{
}

void Rectangle:changeWidth(const unsigned width)
{
  /* Width and Height are independant */
    m_width = width;
    // We should discuss if it is good to mutate 
    // and not create a new Figure
}

void Rectangle::changeHeight(const unsigned height)
{
    m_height = height;
}

unsigned Rectangle::area() const
{ 
    return m_height * m_width;
}

// No inheritance
Square::Square(const unsigned size):
    m_size(size)
{
}

unsigned Square::area() const
{ 
    return m_size * m_size;
}
 
void Square::changeSize(const unsigned size)
{
    m_size = size; 
}
 
void testRectangleChange(Rectangle& r)
{
    unsigned h = 10;
    auto w = r.getWidth();
    r.setHeight(h);

    std::cout << "Expected area: " << (w*h) << ", got " << r.area() << "\n";
    // area is not well defined in squares
    // every square IS-A rectangle, but does not behave-like a rectangle
}

int main()
{
    Rectangle rectangle{3,4};
    Square square{5};
    testRectangleChange(rectangle);
    testRectangleChange(square);
} Detection Manual [x] This is a semantic smell. Tags Inheritance Conclusion Number IS-A number (according to math). Real Complex IS-A number (according to math). Integer Real Number does not Behave-Like-A number. Real Complex We cannot do real.setImaginaryPart() so it is not a Complex according to our Bijection Relations Code Smell 92 - Isolated Subclasses Names Code Smell 11 - Subclassification for Code Reuse Code Smell 37 - Protected Attributes More Info Circle/Ellipse Problem Subtyping No Silver Bullet Bijection Principle Credits Photo by on Joshua Rondeau Unsplash DRY - Don't Repeat Yourself - Every piece of knowledge must have a single, unambiguous, authoritative representation within a system. Andy Hunt Software Engineering Great Quotes And that’s all for now… The next article will explain 5 more code smells!

Too Long; Didn't Read

Making security fun one episode at a time

How to Find the Stinky Parts of Your Code [Part XXV]

Too Long; Didn't Read

Maximiliano Contieri

About Author

TOPICS

THIS ARTICLE WAS FEATURED IN...

How to Find the Stinky Parts of Your Code [Part XXV]

Too Long; Didn't Read

Maximiliano Contieri

About Author

TOPICS

THIS ARTICLE WAS FEATURED IN...

RELATED STORIES