Code Smell 134 - Specialized Business Collections

Maxi Contieri - May 22 '22 - - Dev Community

If it walks like a duck and it quacks like a duck, then it must be a duck

TL;DR: Don't create unnecessary abstractions

Problems

  • Over Design

  • Unneeded classes

Solutions

  1. Use a standard class

Context

Discovering abstractions on the MAPPER is a hard task.

After refining we should remove unneeded abstractions.

Sample Code

Wrong

<?php

Namespace Spelling;

final class Dictionary {

    private $words;
    function __construct(array $words) {
        $this->words = $words;
    }

    function wordsCount(): int {
        return count($this->words);
    }

    function includesWord(string $subjectToSearch): bool {
        return in_array($subjectToSearch, $this->words);
    }
}

//This has protocol similar to an abstract datatype dictionary
//And the tests

use PHPUnit\Framework\TestCase;

final class DictionaryTest extends TestCase {
    public function test01EmptyDictionaryHasNoWords() {
        $dictionary = new Dictionary([]);
        $this->assertEquals(0, $dictionary->wordsCount());
    }

    public function test02SingleDictionaryReturns1AsCount() {        
        $dictionary = new Dictionary(['happy']);
        $this->assertEquals(1, $dictionary->wordsCount());
    }

    public function test03DictionaryDoesNotIncludeWord() {
        $dictionary = new Dictionary(['happy']);
        $this->assertFalse($dictionary->includesWord('sadly'));
    }

    public function test04DictionaryIncludesWord() {
        $dictionary = new Dictionary(['happy']);
        $this->assertTrue($dictionary->includesWord('happy'));
    }
} 

Enter fullscreen mode Exit fullscreen mode

Right

<?php

Namespace Spelling;

// final class Dictionary is no longer needed

//The tests use a standard class 
//In PHP we use associative arrays
//Java an other languages have HashTables, Dictionaries etc. etc.

use PHPUnit\Framework\TestCase;

final class DictionaryTest extends TestCase {
    public function test01EmptyDictionaryHasNoWords() {
        $dictionary = [];
        $this->assertEquals(0, count($dictionary));
    }

    public function test02SingleDictionaryReturns1AsCount() {
        $dictionary = ['happy']; 
        $this->assertEquals(1, count($dictionary));
    }

    public function test03DictionaryDoesNotIncludeWord() {
        $dictionary = ['happy']; 
        $this->assertFalse(in_array('sadly', $dictionary));
    }

    public function test04DictionaryIncludesWord() {
        $dictionary = ['happy'];  
        $this->assertTrue(in_array('happy', $dictionary));
    }
} 

Enter fullscreen mode Exit fullscreen mode

Detection

[X] SemiAutomatic

Based on protocols, we should remove some unnecessary classes

Tags

  • Protocols

Exceptions

Sometimes we need to optimize collections for performance reasons if we have enough strong evidence.

Conclusion

We need to clean up code from time to time.

Specialized collections are a good starting point.

Relations

More Info

Credits

Photo by Pisit Heng on Unsplash


Most of the effort in the software business goes into the maintenance of code that already exists.

Wietse Venema


This article is part of the CodeSmell Series.

. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .