Skip to main content

Code Smells

aka anti-patterns

Table of Contents

God Object: A class or component that tries to do too much and violates the principles of single responsibility. Spaghetti Code: Unorganized and tangled code with poor structure, making it difficult to understand and maintain. Tight Coupling: Components or modules that have strong dependencies on each other, making it challenging to change or replace one without affecting others. Duplicate Code: Repeating blocks of code across the application, leading to maintenance issues and increasing the likelihood of bugs. Magic Numbers/Strings: Using hard-coded numeric or string values directly in the code instead of defining them as constants or configuration variables. Lack of Input Validation: Failing to properly validate and sanitize user input, leaving the application vulnerable to security risks like SQL injection or cross-site scripting (XSS). Overly Complex Logic: Writing convoluted and hard-to-understand code, making it difficult to maintain and debug.

https://pragmaticways.com/31-code-smells-you-must-know/

Code Smells Within ClassesDefinition
Conditional Complexitylimit the ability to extend and provide re-use. Watch out for large conditional logic blocks, particularly blocks that tend to grow larger or change significantly over time. Consider alternative object-oriented approaches such as decorator, strategy, or state.
Combinatorial ExplosionYou have lots of code that does almost the same thing.. but with tiny variations in data or behavior. This can be difficult to refactor-- perhaps using generics or an interpreter?
Type Embedded in NameAvoid placing types in method names; it's not only redundant, but it forces you to change the name if the type changes.
Uncommunicative NameDoes the name of the method succinctly describe what that method does? Could you read the method's name to another developer and have them explain to you what it does? If not, rename it or rewrite it.
Inconsistent NamesPick a set of standard terminology and stick to it throughout your methods. For example, if you have Open(), you should probably have Close().
Temporary FieldWatch out for objects that contain a lot of optional or unnecessary fields. If you're passing an object as a parameter to a method, make sure that you're using all of it and not cherry-picking single fields.

Table of Contents ⬆️

Code Smells Between ClassesDefinition
Circular Referencesome common causes. Objects generated through constructor injection that reference each other in their constructor parameters. Objects generated through constructor injection where an instance of a class is passed as a parameter to its own constructor. Objects generated through method call injection that reference each other. Objects generated through property (setter) injection that reference each other
Alternative Classes with Different InterfacesIf two classes are similar on the inside, but different on the outside, perhaps they can be modified to share a common interface.
Data ClassAvoid classes that passively store data. Classes should contain data and methods to operate on that data, too.
Data ClumpsIf you always see the same data hanging around together, maybe it belongs together. Consider rolling the related data up into a larger class.
Refused BequestIf you inherit from a class, but never use any of the inherited functionality, should you really be using inheritance?
Inappropriate IntimacyWatch out for classes that spend too much time together, or classes that interface in inappropriate ways. Classes should know as little as possible about each other.
Indecent ExposureBeware of classes that unnecessarily expose their internals. Aggressively refactor classes to minimize their public surface. You should have a compelling reason for every item you make public. If you don't, hide it.
Feature EnvyMethods that make extensive use of another class may belong in another class. Consider moving this method to the class it is so envious of.
Message ChainsWatch out for long sequences of method calls or temporary variables to get routine data. Intermediaries are dependencies in disguise.
Middle ManIf a class is delegating all its work, why does it exist? Cut out the middleman. Beware classes that are merely wrappers over other classes or existing functionality in the framework.
Divergent ChangeIf, over time, you make changes to a class that touch completely different parts of the class, it may contain too much unrelated functionality. Consider isolating the parts that changed in another class.
Shotgun SurgeryIf a change in one class requires cascading changes in several related classes, consider refactoring so that the changes are limited to a single class.
Parallel Inheritance HierarchiesEvery time you make a subclass of one class, you must also make a subclass of another. Consider folding the hierarchy into a single class.
Incomplete Library ClassWe need a method that's missing from the library, but we're unwilling or unable to change the library to include the method. The method ends up tacked on to some other class. If you can't modify the library, consider isolating the method.
Solution SprawlIf it takes five classes to do anything useful, you might have solution sprawl. Consider simplifying and consolidating your design.

Table of Contents ⬆️

asp.net

When an app grows too big

Small App (~1000's LOC)

  • Tiny, Fast
  • Easy to develop
  • Testing is trivial
  • Simple project structure works

Large App (~10k-50k LOC)

  • unmaintainable
  • prone to bugs
  • testing requires thought
  • performance

BLOATERS

Bloater Code Smells occur when code is way bigger than it should be, but you don't exactly know "when" it happened, it just kind of creeps up on you. Usually, the project started out with some rock solid code, but as the life of the program continues to age, new requirements come in, and different programmers cycle through the code base, the code smells start trickling in as more and more code gets added to the same old classes. No one honors the Programmer Boy Scout Rule -- leave the code base camp cleaner than you found it!

Many of the common examples of bloater code smells include:

  • Classes that are entirely too big (Large Class)
  • Methods and functions that are also entirely too big (Long Method)
  • Way too many parameters for a given function (Long Parameter List)
  • Too many primitives and constants that should be encapsulated together in an object (Primitive Obsession)
  • Same group of data being repeated in several different places (Data Clump)

Large Class

large class

  • Large classes, like long methods, are difficult to read, understand, and troubleshoot. Does the class contain too many responsibilities? Can the large class be restructured or broken into smaller classes?

Long Method

long method

  • Aim for an average of 5 lines of code, but no longer than 10 lines.
  • All other things being equal, a shorter method is easier to read, easier to understand, and easier to troubleshoot. Refactor long methods into smaller methods if you can.

Long Parameter

long param list

  • List The more parameters a method has, the more complex it is. Limit the number of parameters you need in a given method, or use an object to combine the parameters.

Primitive Obsession

primitive > class

  • giving up on the dynamic specialized purpose of a class
  • Don't use a gaggle of primitive data type variables as a poor man's substitute for a class. If your data type is sufficiently complex, write a class to represent it.

DISPENSABLES

Dispensable Code Smells are generally unnecessary code that should be removed from the source. They range in the form of:

  • Comments
  • Duplicate Code
  • Lazy Class
  • Dead Code
  • Speculative Generality
  • Oddball Solution

Comments

comments

  • There's a fine line between comments that illuminate and comments that obscure. Are the comments necessary? Do they explain "why" and not "what"? Can you refactor the code so the comments aren't required? And remember, you're writing comments for people, not machines.

Duplicated code

dupe

  • very obvious or more subtle cases of near-duplication.
  • Don't Repeat Yourself!

Lazy Class

lazy class

  • they don't do much on their own and should be consolidated
  • Classes should pull their weight. Every additional class increases the complexity of a project. If you have a class that isn't doing enough to pay for itself, can it be collapsed or combined into another class?

Dead Code

delete dead code

  • Ruthlessly delete code that isn't being used. That's why we have source control systems!

Speculative Generality

speculative gen

  • Write code to solve today's problems, and worry about tomorrow's problems when they actually materialize. Everyone loses in the "what if.." school of design. You (Probably) Aren't Gonna Need It.

Oddball Solution

oddball

  • There should only be one way of solving the same problem in your code. If you find an oddball solution, it could be a case of poorly duplicated code-- or it could be an argument for the adapter model, if you really need multiple solutions to the same problem.