Say it only once
Problems
Bad Responsibilities Assignments
Code Duplication
Maintainability
Single Responsibility Violation.
Copy-pasted code.
Solutions
- Refactor
Sample Code
Wrong
<?
final class SocialNetwork {
function postStatus(string $newStatus) {
if (!$user->isLogged()) {
throw new Exception('User is not logged');
}
///...
}
function uploadProfilePicture(Picture $newPicture) {
if (!$user->isLogged()) {
throw new Exception('User is not logged');
}
///...
}
function sendMessage(User $recipient, Message $messageSend) {
if (!$user->isLogged()) {
throw new Exception('User is not logged');
}
///...
}
}
Right
<?
final class SocialNetwork {
function postStatus(string $newStatus) {
$this->assertUserIsLogged();
///...
}
function uploadProfilePicture(Picture $newPicture) {
$this->assertUserIsLogged();
///...
}
function sendMessage(User $recipient, Message $messageSend) {
$this->assertUserIsLogged();
///...
}
function assertUserIsLogged() {
if (!$this->user->isLogged()) {
throw new Exception('User is not logged');
//This is just a simplification to show the code smell
//Operations should be defined as objects with preconditions etc.
}
}
}
Detection
Some modern linters can detect repeated patterns (not just repeated code) and also while performing our code reviews we can easily detect this problem and ask for a refactor.
Tags
- Code Duplication
Conclusion
Adding a new feature should be straightforward it our model maps 1:1 to real world and our responsibilities are in the correct places.
We should be alert for small changes spanning in several classes.
More info
Credits
Photo by William Isted on Unsplash
Duplication is the primary enemy of a well-designed system.
Robert Martin
This article is part of the CodeSmell Series.
Latest comments (5)
I see two same snippets. Was looking for a difference for 5 minutes, feeling dumb, now I'm amused after seeing they are the same file 😆
it is a bug in devto's cache. i am migrating my articles to inline code right now
For me, right now, both gists are linked to shotgun.php.
I strongly suggest you use markdown code blocks with syntax highlighting instead, especially for such short snippets. They are more lightweight, require less work, support theming (don't blind people, despite having dark theme both here and on github the gist embed is a brilliant white) and avoid bugs such as swapped-gist-order. There is almost no excuse to ever use a gist on DEV.to, this is not Medium.
Hi
There is a bug in dev.to mobile app related to caches (another code smell)
Try refreshing it.
I upload code in Gists since I am sharing it with other articles.
Nevertheless I am working in an automatic migration to embed and use the syntax as you pointed out
I was on web, with clean cache. It's fine now (still gists, but in correct order)