DEV Community

Cover image for Code Smell 82 - Tests Violating Encapsulation
Maxi Contieri
Maxi Contieri

Posted on • Updated on • Originally published at maximilianocontieri.com

Code Smell 82 - Tests Violating Encapsulation

Objects work fine and fulfill business objectives. But we need to test them. Let's break them.

TL;DR: Don't write methods with the only purpose of being used in your tests.

Problems

  • Encapsulation Violation.

  • Bad interfaces

  • Coupling

Solutions

  1. Don't break encapsulation.

  2. Test must be in full control.

  3. If you cannot control your object, you are coupled. Decouple them!

Sample Code

Wrong

<?

class Hangman {
    private $wordToGuess;

    function __construct() {
        $this->wordToGuess = getRandomWord();
        //Test is not in control of this
    }

    public function getWordToGuess(): string {
        return $this->wordToGuess;
    }
}

class HangmanTest extends TestCase {
    function test01WordIsGuessed() {
        $hangmanGame = new Hangman();
        $this->assertEquals('tests', $hangmanGame->wordToGuess());
        //how can we make sure the word is guessed?
    }
}
Enter fullscreen mode Exit fullscreen mode

Right

<?

class Hangman {
    private $wordToGuess;

    function __construct(WordRandomizer $wordRandomizer) {
        $this->wordToGuess = $wordRandomizer->newRandomWord();
    }
}

class MockRandomizer implements WordRandomizer {
    function newRandomWord(){
        return 'tests';
    }
}

class HangmanTest extends TestCase {
    function test01WordIsGuessed() {
        $hangmanGame = new Hangman(new MockRandomizer());
        $this->assertFalse($hangmanGame->wordWasGuessed());
        $hangmanGame->play('t');
        $this->assertFalse($hangmanGame->wordWasGuessed());
        $hangmanGame->play('e');
        $this->assertFalse($hangmanGame->wordWasGuessed());
        $hangmanGame->play('s');
        $this->assertTrue($hangmanGame->wordWasGuessed());
        //We just test behavior
    }
}
Enter fullscreen mode Exit fullscreen mode

Detection

This is a design smell.

We can detect we need a method just for test.

Tags

  • Information Hiding

Conclusion

White-box tests are fragile. They test implementation instead of behavior.

Relations

More Info

Credits

This smell was inspired by @Rodrigo


Nothing makes a system more flexible than a suite of tests.

Robert Martin


This article is part of the CodeSmell Series.

Oldest comments (5)

Collapse
 
mcsee profile image
Maxi Contieri

Hi!!

Sorry I don't use Skype. I consider it too fat and bloated.
Please DM me on twitter @mcsee1

Collapse
 
yoursunny profile image
Junxiao Shi

Is it Hangman (implementation) or Hagnman (in test case)?

Collapse
 
mcsee profile image
Maxi Contieri • Edited

i don't undestand the question.

Test creates a new hangman game

´´´
$hangmanGame = new Hangman
´´´

Collapse
 
yoursunny profile image
Junxiao Shi

Test creates an instance of Hagnman, not Hangman.

 $hangmanGame = new Hagnman(new MockRandomizer());
Enter fullscreen mode Exit fullscreen mode
Thread Thread
 
mcsee profile image
Maxi Contieri

corrected ! thank you very much!