Java Code Smells and Refactoring Guide
Java Code Smells and Refactoring Guide
Contents
2 Object-Orientation Abusers 6
2.1 Alternative Classes with Different Interfaces . . . . . . . . . . . . . . . . . . . . . 6
2.2 Refused Bequest . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 6
2.3 Switch Statements . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 7
2.4 Temporary Field . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 8
3 Change Preventers 9
3.1 Divergent Change . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 9
3.2 Parallel Inheritance Hierarchies . . . . . . . . . . . . . . . . . . . . . . . . . . . . 9
3.3 Shotgun Surgery . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 10
4 Dispensables 12
4.1 Comments . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 12
4.2 Duplicate Code . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 12
4.3 Data Class . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 13
4.4 Dead Code . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 13
4.5 Lazy Class . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 14
4.6 Speculative Generality . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 14
5 Couplers 16
5.1 Feature Envy . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 16
5.2 Inappropriate Intimacy . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 16
5.3 Incomplete Library Class . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 17
5.4 Middle Man . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 18
5.5 Message Chains . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . 18
1
1 BLOATERS (BLOATED CODE) 2
Refactored Code:
1 public class Calculator {
2 public void calculate ( int a , int b , String operation ) {
3 int result = performOperation (a , b , operation ) ;
4 printResult ( result ) ;
5 }
6
7 private int performOperation ( int a , int b , String operation ) {
8 if ( operation . equals ( " add " ) ) return a + b ;
9 if ( operation . equals ( " subtract " ) ) return a - b ;
10 if ( operation . equals ( " multiply " ) ) return a * b ;
11 return 0;
12 }
13
14 private void printResult ( int result ) {
15 System . out . println ( " Result : " + result ) ;
16 }
17 }
Solution: Break down the method into smaller methods, each performing a single respon-
sibility.
Refactored Code:
1 public class Email {
2 private final String value ;
3
4 public Email ( String email ) {
5 if ( email == null || ! email . contains ( " @ " ) ) {
6 throw new I ll e g al A r gu m e nt E x ce p t io n ( " Invalid email " ) ;
7 }
8 this . value = email ;
9 }
10
11 public String getValue () {
12 return value ;
13 }
14 }
15
16 public class Person {
17 private Email email ;
18
19 public void setEmail ( Email email ) {
20 this . email = email ;
21 }
22 }
Solution: Create value objects to represent domain concepts instead of using primitives.
Refactored Code:
1 public class Name {
2 private String firstName ;
3 private String lastName ;
4
5 public Name ( String firstName , String lastName ) {
6 this . firstName = firstName ;
7 this . lastName = lastName ;
8 }
9 }
10
11 public class Address {
12 private String street ;
Solution: Group related data into classes or structs to reduce parameter lists and improve
maintainability.
Refactored Code:
1 public class Student {
2 private String name ;
3 private int age ;
4 private String email ;
5 }
6
7 public class GradeCalculator {
8 public void calculateGrade ( Student student ) { }
9 }
10
11 public class StudentRepository {
12 public void save ( Student student ) { }
13 }
14
15 public class ReportGenerator {
16 public void generateReport ( Student student ) { }
17 }
Solution: Split the class into smaller classes, each with a clear and single responsibility.
Refactored Code:
1 public class UserInfo {
2 private String firstName ;
3 private String lastName ;
4 private String email ;
5 private String phone ;
6 private String address ;
7 }
8
9 public class UserService {
10 public void createUser ( UserInfo userInfo ) {
11 }
12 }
Solution: Group related parameters into an object or class to reduce the number of param-
eters.
2 Object-Orientation Abusers
All smells in this group are incomplete or incorrect application of object-oriented programming
principles.
Refactored Code:
1 public interface Shape {
2 int getArea () ;
3 }
4
5 public class Rectangle implements Shape {
6 private int width ;
7 private int height ;
8
9 @Override
10 public int getArea () {
11 return width * height ;
12 }
13 }
14
15 public class Square implements Shape {
16 private int side ;
17
18 @Override
19 public int getArea () {
20 return side * side ;
21 }
22 }
10 }
11 }
Refactored Code:
1 public class Bird {
2 public void eat () { }
3 }
4
5 public interface Flyable {
6 void fly () ;
7 }
8
9 public class Sparrow extends Bird implements Flyable {
10 @Override
11 public void fly () { }
12 }
13
14 public class Penguin extends Bird {
15 }
Refactored Code:
1 public interface PaymentMethod {
2 void process ( double amount ) ;
3 }
4
5 public class CreditCardPayment implements PaymentMethod {
6 @Override
7 public void process ( double amount ) { }
8 }
9
10 public class PayPalPayment implements PaymentMethod {
11 @Override
12 public void process ( double amount ) { }
13 }
14
15 public class PaymentProcessor {
16 public void process ( PaymentMethod method , double amount ) {
17 method . process ( amount ) ;
18 }
19 }
Solution: Use polymorphism, strategy pattern, or state pattern to replace switch state-
ments.
Refactored Code:
1 public class Order {
2 private double total ;
3
4 public void calculateTotal () {
5 total = 100;
6 if ( hasPromotion () ) {
7 total = applyDiscount ( total ) ;
8 }
9 }
10
11 private double applyDiscount ( double amount ) {
12 return amount * 0.9;
13 }
14
15 private boolean hasPromotion () {
16 return false ;
17 }
18 }
Solution: Extract the logic that uses the field into a separate class, or use a parameter
object.
3 Change Preventers
These smells mean that if you need to change something in one place in your code, you have to
make many changes in other places too. Program development becomes much more complicated
and expensive as a result.
Refactored Code:
1 public class Employee {
2 private String name ;
3 private double salary ;
4 }
5
6 public class SalaryCalculator {
7 public double calculateSalary ( Employee employee ) {
8 return employee . getSalary () * 1.1;
9 }
10 }
11
12 public class EmployeeRepository {
13 public void save ( Employee employee ) { }
14 }
15
16 public class ReportGenerator {
17 public void generateReport ( Employee employee ) { }
18 }
Solution: Split the class into smaller classes, each changing for only one reason.
Refactored Code:
Refactored Code:
1 public enum OrderStatus {
2 PENDING , CANCELLED ;
3
4 public boolean canProcess () {
5 return this != CANCELLED ;
6 }
7 }
8
9 public class Order {
10 private OrderStatus status ;
11
12 public boolean isCancelled () {
13 return status == OrderStatus . CANCELLED ;
14 }
15 }
16
17 public class OrderValidator {
18 public boolean validate ( Order order ) {
19 return ! order . isCancelled () ;
20 }
21 }
22
23 public class OrderProcessor {
24 public void process ( Order order ) {
25 if ( order . isCancelled () ) {
26 return ;
27 }
28 }
29 }
Solution: Move related changes into a single class, use Move Method or Move Field refac-
toring techniques.
4 Dispensables
A dispensable is something pointless and unneeded whose absence would make the code cleaner,
more efficient, and easier to understand.
4.1 Comments
Too many comments or comments explaining complex code instead of making the code self-
explanatory.
Code with Smell:
1 public class Calculator {
2 // This method adds two numbers
3 public int add ( int a , int b ) {
4 return a + b ; // Return the sum
5 }
6
7 // Check if number is even
8 public boolean isEven ( int number ) {
9 return number % 2 == 0; // If remainder is 0 , it ’s even
10 }
11 }
Refactored Code:
1 public class Calculator {
2 public int add ( int a , int b ) {
3 return a + b ;
4 }
5
6 public boolean isEven ( int number ) {
7 return number % 2 == 0;
8 }
9 }
Solution: Make code self-documenting, only comment when truly necessary (e.g., explaining
complex algorithms).
Refactored Code:
1 public class OrderService {
2 public void processOrder ( Order order ) {
3 validateOrder ( order ) ;
4 }
5
6 public void cancelOrder ( Order order ) {
7 validateOrder ( order ) ;
8 }
9
10 private void validateOrder ( Order order ) {
11 if ( order == null ) {
12 throw new I ll e g al A r gu m e nt E x ce p t io n ( " Order cannot be null " ) ;
13 }
14 }
15 }
Refactored Code:
1 public class Order {
2 private List < OrderItem > items ;
3
4 public double getTotal () {
5 return items . stream ()
6 . mapToDouble ( item -> item . getPrice () * item . getQuantity () )
7 . sum () ;
8 }
9 }
Solution: Move behavior into the class, or create methods with meaningful business logic.
3
4 public void updateUser ( String name ) { }
5
6 public void deleteUser ( String name ) {
7 // Never called
8 }
9
10 private String unusedField = " test " ;
11 }
Refactored Code:
1 public class UserService {
2 public void createUser ( String name ) { }
3
4 public void updateUser ( String name ) { }
5 }
Solution: Remove unused code, use code analysis tools to identify dead code.
Refactored Code:
1 public class User {
2 private String name ;
3
4 public String getName () {
5 return name != null ? name . trim () : " " ;
6 }
7 }
Solution: Inline Class, merge into another class, or add behavior if needed.
Refactored Code:
1 public class CreditCardProcessor {
2 public void process ( CreditCard payment ) { }
3 }
Solution: Simplify code, only implement what is truly necessary. Refactor to create ab-
straction when needed.
5 Couplers
All smells in this group contribute to excessive coupling between classes or show what happens
if coupling is replaced by excessive delegation.
Refactored Code:
1 public class Order {
2 private Customer customer ;
3 private List < OrderItem > items ;
4
5 public String getSummary () {
6 return customer . getName () + " - " + items . size () + " items " ;
7 }
8 }
9
10 public class OrderPrinter {
11 public void print ( Order order ) {
12 System . out . println ( order . getSummary () ) ;
13 }
14 }
Solution: Move the method to the class that contains the data used most.
Refactored Code:
Refactored Code:
1 public interface EmailService {
2 void send ( String to , String subject , String body ) ;
3 void sendWithAttachment ( String to , String subject ,
4 String body , String attachment ) ;
5 }
6
7 public class EmailServiceAdapter implements EmailService {
8 private ExternalEmailService externalService ;
9
10 @Override
11 public void send ( String to , String subject , String body ) {
12 externalService . send ( to , subject , body ) ;
13 }
14
15 @Override
16 public void sendWithAttachment ( String to , String subject ,
17 String body , String attachment ) {
18 // Implementation
19 }
20 }
Solution: Use wrapper class, extension methods, or adapter pattern to extend library func-
tionality.
Refactored Code:
1 public class Person {
2 private Department department ;
3
4 public Department getDepartment () {
5 return department ;
6 }
7 }
8
9 // Remove PersonManager , use Person directly
Solution: Inline Class, remove the middle man and call directly.
Refactored Code:
1 public class Person {
2 private Department department ;
3
4 public String getCompanyName () {
5 return department != null ? department . getCompanyName () : null ;
6 }
7 }
8
9 public class Department {
10 private Company company ;
11
12 public String getCompanyName () {
13 return company != null ? company . getName () : null ;
14 }
15 }
16
17 public class PersonService {
18 public String getCompanyName ( Person person ) {
19 return person . getCompanyName () ;
20 }
21 }
Solution: Hide Delegate, create method wrapper, or use Law of Demeter to reduce chain
length.