How shall we write get method, so that private fields don't escape their intended scope?

Question

From Java Concurrency in Practice

Publishing one object may indirectly publish others. If you add a Secret to the published knownSecrets set, you’ve also published that Secret, because any code can iterate the Set and obtain a reference to the new Secret. Similarly, returning a reference from a nonprivate method also publishes the returned object. UnsafeStates in Listing 3 . 6 publishes the supposedly private array of state abbreviations.
class UnsafeStates {    
    private String[] states = new String[] {        
        "AK", "AL" ...        
    };    
    public String[] getStates() { return states; }    
}
Publishing states in this way is problematic because any caller can modify its contents. In this case, the states array has escaped its intended scope, because what was supposed to be private state has been effectively made public.

In the example, what is the correct way to write getStates method, so that the states array doesn't escape its intended scope?

Is it correct that the quote about escape isn't directly related to multithreading and synchronization?

Thanks.

score 1 · Answer 1 · answered Oct 13 '17 at 22:32

1

You can use Java's Arrays and Collections. to create a read only wrapper.

return Collections.unmodifiableList(Arrays.asList(states));

NOTE: don't worry about the Strings getting modified, they're immutable.

answered Oct 13 '17 at 22:32

user3414321

421
1
4
11

1

Thanks. Does the problem in my post only apply to reference types but not to value types? – Oct 13 '17 at 23:13
Yes, just don't forget arrays of primitive value types are reference types too. – user3414321 Oct 13 '17 at 23:16

score -2 · Answer 2 · answered Oct 13 '17 at 22:37

-2

class SafeStates {

  private List<String> states;

  public List<String> getStates() {
    return states;
  }

  public void setStates(List<String> states) {
    this.states = Collections.unmodifiableList(states);
  }
}

answered Oct 13 '17 at 22:37

Dmitry

87
4

This class is not trustworthy. Way too easy to manipulate the `states` without SafeStates being able to prevent that. – Tom Oct 13 '17 at 22:38
getStates()[0] = "evil"; modifies. – djechlin Oct 13 '17 at 22:41
modification will not be possible: UnsupportedOperationException will be thrown – Dmitry Oct 13 '17 at 22:43
1

please read: https://docs.oracle.com/javase/7/docs/api/java/util/Collections.html#unmodifiableList(java.util.List) – Dmitry Oct 13 '17 at 22:48
Have you read the linked Java Doc? Do you know what "view" means? That posted example thinks that no one will or can modify the list passed to `setStates`, which is obviously not true. – Tom Oct 14 '17 at 00:42
General idea is to hold unmodifiable list inside instance instead of creating it each time whenever getter is being called (which will obviously decrease performance). Setter could be replaced by any code which loads states into memory. It used here just for example because hardcoded valves is not good, and real use case of this code is unknown. – Dmitry Oct 14 '17 at 03:02
No one is talking about hardcoded values. Your code fails to cut the link between the passed list and the internal used one. Therefore the "immutability" is just a façade. – Tom Oct 14 '17 at 09:26

How shall we write get method, so that private fields don't escape their intended scope?

2 Answers2

Linked

Related