Skip to main content

Command Palette

Search for a command to run...

Code Smell 46 - Repeated Code

Published
2 min read
Code Smell 46 - Repeated Code
M

I’m a senior software engineer loving clean code, and declarative designs. S.O.L.I.D. and agile methodologies fan.

DRY is our mantra. Teachers tell us to remove duplication. We need to go beyond.

Problems

Solutions

  1. Find repeated patterns (not repeated code).

  2. Create an abstraction.

  3. Parametrize abstraction calls.

  4. Use composition and never inheritance.

  5. Unit test new abstraction.

Sample Code

Wrong

<?

final class WordProcessor {

    function replaceText(string $patternToFind, string $textToReplace) {
        $this->text = '<<<' . str_replace($patternToFind, $textToReplace, $this->text) . '>>>';
    }
}

final class Obfuscator {

    function obfuscate(string $patternToFind, string $textToReplace) {
        $this->text = strlower(str_ireplace($patternToFind, $textToReplace, $this->text));
    }
}

Right

<?

final class TextReplacer {
    function replace(string $patternToFind, string $textToReplace, string $subject, string $replaceFunctionName, $postProcessClosure) {
        return $postProcessClosure($replaceFunctionName($patternToFind, $textToReplace, $subject));
    }
}

// Lots of tests on text replacer so we can gain confidence.

final class WordProcessor {

    function replaceText(string $patternToFind, string $textToReplace) {
        $this->text = (new TextReplacer())->replace($patternToFind, $textToReplace, $this->text, 'str_replace', fn($text) => '<<<' . $text . '>>>');
    }
}

final class Obfuscator {

    function obfuscate(string $patternToFind, string $textToReplace) {
        $this->text = (new TextReplacer())->replace($patternToFind, $textToReplace, $this->text, 'str_ireplace', fn($text) => strlower($text));
    }
}

Detection

Linters can find repeated code.

There are not very good finding similar patterns.

Maybe soon machine learning will help us find such abstractions automatically.

For now, it is up to us, humans.

Examples

Tags

  • Duplication

Conclusion

Repeated code is always a smell.

Copying and pasting code is always a shame.

With our refactoring tools, we need to accept the duplication remove challenge trusting our tests as a safety net.

Relations

More info

Credits

Photo by Sid Balachandran on Unsplash


Copy and paste is a design error.

David Parnas



This article is part of the CodeSmell Series.

B
BarTech5y ago

Full agreement with general idea. But given example that should fix supposed duplication is a regression from readability perspective. TextReplacer()->replace() API is awful. First it's more code not less and most important it's way harder to understand. I would classify this as clever code or overengineering. Refactoring like this make sense when repeated code have more logic, some local variables, in general you have to spend time to understand what it's doing. In this case abstracting and giving method descriptive name make total sense. In your example it's making simple code complicated.

4
M

I agree with you. It is an over engineer example. I tried to find a better one and also shorter. Many examples i find in real world are too long to embed.

I might replace it with a better one soon.

Code Smells

Part 1 of 50

In this series, we will see several symptoms and situations that make us doubt the quality of our developments. We will present possible solutions. Most are just clues. They are no hard rules.