DEV Community

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

Posted on • Edited 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.

Top comments (5)

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!

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