Getters and setters are not evil per se, they're evil when they're used for things they shouldn't be used for.
As there are cases when using a getter is inevitable, there are also cases when using the Tell Don't Ask principle is much better suited as a solution for the problem rather than a get*
method.
Why are getters exactly frown upon then?
It's quite simple. They break encapsulation by giving you access to protected
and private
variables of a class, and encapsulation is one of the main benefits of OO design.
And when encapsulation is broken and getters are abused, you are very likely to switch from the object oriented design, where components are wired together, to the concept, where you tear an entire object apart by get
ting its values and doing operations based on these, otherwise private, values - you're back to procedural programming.
But as with everything, there are cases where getters are not only a good solution but actually the only one.
For example an entire ORM layer: abstracting pure database to objects, which simply need to have getters. They are simple data transfer objects whose sole responsibility is to deliver data, not to provide any logic. They provide data through getters and setters while usually also map their properties to concrete database attributes (columns if you wish). You use these very simple structures to build domain entities (domain models).
It is not recommended for domain models to have getters/setters
You should layer your application in a way where there is a layer of objects, which even though have getters, only use the getters to provide data for the presentation or persistence layers (that is if you want to display or persist a domain model, you extract its attributes using getters and forward them to the presentation or persisting layer respectively). In no way should the getter be used to pull a value out of the object and do an operation on it.
Take a look at this example (written in PHP, the language I am the most comfortable with):
class HarmlessMonster
{
/** @var int */
private $health;
/** @var int */
private $armor;
/**
* HarmlessMonster constructor.
* @param int $health
* @param int $armor
*/
public function __construct($health, $armor)
{
$this->health = $health;
$this->armor = $armor;
}
/**
* @param int $health
*/
public function setHealth($health)
{
$this->health = $health;
}
/**
* @return int
*/
public function getHealth()
{
return $this->health;
}
/**
* @return int
*/
public function getArmor()
{
return $this->armor;
}
}
class Player
{
/** @var int */
private $damage;
/** @var int */
private $weaponWearPercentage;
/**
* Player constructor.
* @param int $damage
* @param int $weaponWearPercentage
*/
public function __construct($damage, $weaponWearPercentage = 0)
{
$this->damage = $damage;
$this->weaponWearPercentage = $weaponWearPercentage;
}
/**
* @return int
*/
public function getDamage()
{
return $this->damage;
}
/**
* @return int
*/
public function getWeaponWearPercentage()
{
return $this->weaponWearPercentage;
}
/**
* @param int $weaponWearPercentage
*/
public function setWeaponWearPercentage($weaponWearPercentage)
{
$this->weaponWearPercentage = $weaponWearPercentage;
if ($this->getWeaponWearPercentage() > 100) {
$this->setWeaponWearPercentage(100);
}
}
}
To make these classes somehow work, you need another class to provide the logic, a class like this:
class PlayerMonsterService
{
public function attackMonsterByPlayer(Player $player, HarmlessMonster $monster)
{
if (($player->getDamage() * (100 - $player->getWeaponWearPercentage())) <= $monster->getArmor()) {
$player->setWeaponWearPercentage($player->getWeaponWearPercentage() + 2);
return;
}
$monster->setHealth($monster->getHealth() - ($player->getDamage() * (100 - $player->getWeaponWearPercentage()) - $monster->getArmor()));
$player->setWeaponWearPercentage($player->getWeaponWearPercentage() + 1);
}
}
You are completely tearing up the domain models, getting values of private attributes to use them in the setters, because there's a clear connection between them. And that's really ugly. As opposed to a Tell Don't Ask models might look like this:
class GameException extends RuntimeException { }
class ArmorTooHighException extends GameException { }
class HarmlessMonster
{
/** @var int */
private $health;
/** @var int */
private $armor;
/**
* HarmlessMonster constructor.
* @param int $health
* @param int $armor
*/
public function __construct($health, $armor)
{
$this->health = $health;
$this->armor = $armor;
}
public function reduceHealth($damage)
{
if ($damage <= $this->armor) {
throw new ArmorTooHighException('Health was not recuded. Insufficient damage.');
}
$this->health -= ($damage - $this->armor);
}
}
class Player
{
/** @var int */
private $damage;
/** @var int */
private $weaponWearPercentage;
/**
* Player constructor.
* @param int $damage
* @param int $weaponWearPercentage
*/
public function __construct($damage, $weaponWearPercentage = 0)
{
$this->damage = $damage;
$this->weaponWearPercentage = $weaponWearPercentage;
}
public function attack(HarmlessMonster $monsterToBeAttacked)
{
try {
$monsterToBeAttacked->reduceHealth($this->damage * (100 - $this->weaponWearPercentage));
} catch (ArmorTooHighException $e) {
$this->increaseWeaponWearBy(2);
return;
}
$this->increaseWeaponWearBy(1);
}
public function increaseWeaponWearBy($amountToBeIncreased)
{
$this->weaponWearPercentage += $amountToBeIncreased;
if ($this->weaponWearPercentage > 100) {
$this->weaponWearPercentage = 100;
}
}
}
You can see, that even though I am using private variables of both classes, there are no getters what so ever. Because you don't need them.
But even here remember, both the Player
and HarmlessMonster
classes would probably need getters, for the reasons said before, presenting the data to the user or persisting it. The point of this is not to use the getters to calculate values and based on these values do operations.
Domain layer is the layer which in an ideal case should follow quite strictly the Tell Don't Ask principle, having the tell methods, which throw exceptions on an invalid action. This puts all the logic related to the class an operation is run on to a very specific place and exactly where it belongs (usualy the class itself), rather than it being scattered across many places in your codebase.
I have found out, one of the reasons why some people prefer the getter/setter combination over the other approach is laziness, because coming up with the tell solution usually takes a few minutes longer than writing a simple get and doing with the value whatever they want. And it's not necessarily bad, if you don't mind the procedural approach, but if what you're after is OO design, avoiding getters unless you really need them is definitely a good way to do so.