DEV Community

Cover image for Code Smell 66 - Shotgun Surgery
Maxi Contieri
Maxi Contieri

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

Code Smell 66 - Shotgun Surgery

Say it only once

Problems

  • Bad Responsibilities Assignments

  • Code Duplication

  • Maintainability

  • Single Responsibility Violation.

  • Copy-pasted code.

Solutions

  1. 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');
        }
        ///...
    }
}
Enter fullscreen mode Exit fullscreen mode

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.
        }
    }
}
Enter fullscreen mode Exit fullscreen mode

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

Wikipedia

Refactoring.guru

Ndepend

DZone

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.

Discussion (5)

Collapse
qm3ster profile image
Mihail Malo • Edited on

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.

Collapse
mcsee profile image
Maxi Contieri Author

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

Collapse
qm3ster profile image
Mihail Malo

I was on web, with clean cache. It's fine now (still gists, but in correct order)

Collapse
dakujem profile image
Andrej Rypo

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 😆

Collapse
mcsee profile image
Maxi Contieri Author

it is a bug in devto's cache. i am migrating my articles to inline code right now