Smell Detail

Mutable Data

可变数据

Change Preventers

Modifications to data often lead to unexpected results and hard-to-find bugs, where updating data in one place affects expectations elsewhere in the software.

Define

Introduction

Modifications to data often lead to unexpected results and hard-to-find bugs. When you update data in one place without realizing that another part of the software expects completely different data, a feature fails. If the failure only occurs in rare circumstances, finding the cause becomes even more difficult.

Key Identifiers

  • Variables that can be modified from multiple places
  • Shared mutable state between different parts of the system
  • Data structures that are modified in place rather than replaced
  • Variables used for different purposes at different times
  • Missing encapsulation around data updates
  • Derived data that can be calculated elsewhere

Refactoring Suggestions

  1. Encapsulate Variable: Ensure all data updates go through a few functions to make monitoring easier
  2. Split Variable: Separate variables used for different purposes at different times
  3. Move Statements and Extract Function: Separate side-effect-free code from data update operations
  4. Separate Query from Modifier: Ensure callers don't accidentally call side-effect code
  5. Remove Setting Method: Look for opportunities to reduce variable scope
  6. Replace Derived Variable with Query: Calculate values instead of storing mutable derived data
  7. Combine Functions into Class/Transform: Limit the amount of code that needs to modify variables
  8. Change Reference to Value: Replace entire data structures instead of modifying internal data

Explanation

Mutable data becomes particularly problematic when its scope expands beyond a few lines of code. The risk increases with the scope of the variable. The ideal solution is to make data immutable whenever possible, but when that's not feasible, we should constrain data updates through encapsulation and careful design to minimize the risk of unexpected modifications affecting other parts of the system.

Code Example
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
import java.util.*;


// Example of problematic mutable data
class UserAccount {
    public String name;
    public double balance;
    public List<String> permissions;
    public Map<String, Object> metadata;
    
    public UserAccount(String name, double balance) {
        this.name = name;
        this.balance = balance;
        this.permissions = new ArrayList<>();
        this.metadata = new HashMap<>();
    }
}


// Shared mutable state causing problems
class AccountManager {
    private static List<UserAccount> accounts = new ArrayList<>();
    private static Map<String, UserAccount> accountCache = new HashMap<>();
    
    public static void addAccount(UserAccount account) {
        accounts.add(account);
        accountCache.put(account.name, account);
    }
    
    public static UserAccount getAccount(String name) {
        return accountCache.get(name); // Returns mutable reference
    }
    
    public static List<UserAccount> getAllAccounts() {
        return accounts; // Direct access to mutable list
    }
}


// Multiple classes modifying the same mutable data
class PaymentService {
    public void processPayment(String accountName, double amount) {
        UserAccount account = AccountManager.getAccount(accountName);
        if (account != null) {
            // Direct modification of shared mutable data
            account.balance -= amount;
            account.metadata.put("lastPayment", amount);
            account.metadata.put("lastPaymentTime", new Date());
        }
    }
    
    public void addBonus(String accountName, double bonus) {
        UserAccount account = AccountManager.getAccount(accountName);
        if (account != null) {
            account.balance += bonus; // Another place modifying balance
            account.metadata.put("bonusReceived", true);
        }
    }
}


class SecurityService {
    public void grantPermission(String accountName, String permission) {
        UserAccount account = AccountManager.getAccount(accountName);
        if (account != null) {
            // Modifying shared mutable list
            account.permissions.add(permission);
            account.metadata.put("permissionGrantedAt", System.currentTimeMillis());
        }
    }
    
    public void revokeAllPermissions(String accountName) {
        UserAccount account = AccountManager.getAccount(accountName);
        if (account != null) {
            account.permissions.clear(); // Clearing shared data
            account.metadata.remove("permissionGrantedAt");
        }
    }
}


// Problematic derived data that could be calculated
class ReportService {
    private double totalBalance = 0.0; // Derived data stored as mutable state
    private int activeUserCount = 0;
    
    public void updateTotals() {
        // Manually updating derived values
        totalBalance = 0.0;
        activeUserCount = 0;
        
        for (UserAccount account : AccountManager.getAllAccounts()) {
            totalBalance += account.balance;
            if (!account.permissions.isEmpty()) {
                activeUserCount++;
            }
        }
    }
    
    public double getTotalBalance() {
        return totalBalance; // Could be calculated instead of stored
    }
    
    public int getActiveUserCount() {
        return activeUserCount; // Could be calculated instead of stored
    }
}


// Variable used for different purposes (violates single responsibility)
class DataProcessor {
    public void processUserData(List<UserAccount> accounts) {
        String temp = ""; // Variable reused for different purposes
        
        // First usage: building user names
        for (UserAccount account : accounts) {
            temp += account.name + ",";
        }
        System.out.println("Users: " + temp);
        
        // Second usage: building permission list (dangerous reuse)
        temp = "";
        for (UserAccount account : accounts) {
            for (String permission : account.permissions) {
                temp += permission + ";";
            }
        }
        System.out.println("Permissions: " + temp);
        
        // Third usage: completely different purpose
        temp = String.valueOf(accounts.size());
        System.out.println("Count: " + temp);
    }
}