One broken window invites another
TL;DR: Don't reuse an existing getter to bolt on new business logic from outside the object.
Problems 😔
Solutions 😃
- Add real behavior methods
- Keep comparisons inside object
- Pass collaborators, not primitives
- Reserve getters for rendering
- Follow tell, don't ask
Refactorings ⚙️
https://maximilianocontieri.com/refactoring-027-remove-getters
https://maximilianocontieri.com/refactoring-013-remove-repeated-code
Context 💬
An object exposes a getter for one legitimate reason: some other part of the system needs to read that value, usually to display it.
Getters are a code smell, but this one gets a pass, for now.
Later on, you discover that you need new business logic that depends on the same value.
You already have the getter, so you write a function outside the object that calls it and does the comparison itself, breaking the encapsulation principle.
Someone else needs slightly different logic based on the same value.
They also call the getter and write their own version of the comparison.
Now two places decide what that value means, and neither of them is the object that owns it. Typical.
You didn't add a second getter this time.
You reused the first one, because it was already there.
That's the trap.
The getter existed for one reason, and you let it justify skipping the real fix: a method on the object that answers the question itself, instead of handing out the raw value for every caller to interpret on their own.
Don't break more windows
Sample Code 💻
Wrong 🚫
// Food needs to show its use-by date on the shelf
// label, so useByDate() exists for that one reason.
//
// Later, removeExpiredFood() needs to pull expired
// products, so it reuses useByDate() and compares the
// result to today itself, outside Food.
//
// flagNearExpiryFood() needs almost the same check, so
// it also calls useByDate() and writes its own slightly
// different comparison.
//
// Now two functions decide what "expired" means, and
// neither of them is Food.
class Food {
constructor(name, useByDate) {
this.name = name;
this.useByDateValue = useByDate;
}
useByDate() {
return this.useByDateValue;
}
}
function removeExpiredFood(shelf, today) {
return shelf.filter(
food => food.useByDate() >= today
);
}
function flagNearExpiryFood(
shelf, today, warningDays
) {
return shelf.filter(food => {
const daysLeft = daysBetween(
food.useByDate(), today
);
return daysLeft >= 0 &&
daysLeft <= warningDays;
});
}
Right 👉
// Food still exposes useByDate() for the shelf label.
//
// Being expired is now a question Food answers itself,
// through isExpiredOn(), instead of every external
// function reimplementing the comparison from the
// getter on its own.
class Food {
constructor(name, useByDate) {
this.name = name;
this.useByDateValue = useByDate;
}
useByDate() {
return this.useByDateValue;
}
isExpiredOn(today) {
return this.useByDateValue < today;
}
daysUntilExpiryFrom(today) {
return daysBetween(this.useByDateValue, today);
}
}
function removeExpiredFood(shelf, today) {
return shelf.filter(food => !food.isExpiredOn(today));
}
function flagNearExpiryFood(shelf, today, warningDays) {
return shelf.filter(food => {
const daysLeft = food.daysUntilExpiryFrom(today);
return daysLeft >= 0 && daysLeft <= warningDays;
});
}
Detection 🔍
[X] Manual
This is a design smell, and no linter is coming to save you.
Search for a getter that appears inside if, comparison, or filter expressions in more than one place outside its own class.
If two call sites read the same getter and each writes its own comparison against it, the object is missing a method, and the getter is carrying logic it was never meant to carry.
Exceptions 🛑
The smell appears when you reuse that same getter as a shortcut for business logic instead of adding the method the logic actually belongs to.
Don't point to DTOs as a counterexample. A DTO doesn't excuse this. It just breaks encapsulation on purpose and gives the practice a name.
Level 🔋
[x] Intermediate
Why the Bijection Is Important 🗺️
The rule behind that value is a concept that belongs to the object in the MAPPER, not to whichever function happens to call the getter first.
When you keep that rule inside the object, every caller shares the same bijection between the object and the real-world thing it represents.
When you let each caller reimplement the rule from a getter, you create as many private definitions of that concept as you have call sites, and they drift apart the moment one of them changes.
AI Generation 🤖
AI generators create this smell often.
You ask for a function that needs a value the object already exposes through a getter, and it writes a standalone function around that getter, because that's the smallest diff that satisfies the request.
It won't add a method to the object unless you ask for that explicitly.
AI Detection 🧲
AI can detect it, but only if you point it at the pattern.
Try: "Find getters called from more than one place where the caller performs its own comparison or business rule on the result."
Without that prompt, the code passes tests and looks idiomatic, so most assistants won't flag it on their own.
Try Them! 🛠
Remember: AI Assistants make lots of mistakes
Suggested Prompt: Move the comparison logic from external functions into a real method on the object so callers stop reimplementing it from the getter
Conclusion 🏁
A getter you added for one legitimate reason doesn't grant permission to skip every method that comes after it.
When you find yourself reaching for an existing getter to write new business logic outside the object, stop and add the method instead.
The object already knows the information.
Let it hold the rule too.
Stop treating it like a vending machine that only hands out data to whoever asks nicely.
Relations 👩❤️💋👨
https://maximilianocontieri.com/code-smell-68-getters
https://maximilianocontieri.com/code-smell-89-math-feature-envy
https://maximilianocontieri.com/code-smell-63-feature-envy
https://coderlegion.com/7246/code-smell-01-anemic-models
https://maximilianocontieri.com/code-smell-246-expiration-date
https://maximilianocontieri.com/code-smell-64-inappropriate-intimacy
https://maximilianocontieri.com/code-smell-173-broken-windows
https://maximilianocontieri.com/nude-models-part-ii-getters
https://martinfowler.com/bliki/TellDontAsk.html
Encapsulation
Quote
OOP to me means only messaging, local retention and protection and hiding of state-process.
Alan Kay
Disclaimer 📘
Code Smells are my opinion.
Credits 🙏
Photo by Nathália Rosa on Unsplash
This article is part of the CodeSmell Series.
https://coderlegion.com/10942/how-to-find-the-stinky-parts-of-your-code