Java getter access security
So, we created a simple class with some private class member and an automatically generated getter for it. But the getter actually returned a reference to that member, resulting in full access to the private member. Things are good? Here's the class code:
public class User {
private ArrayList<String> strings = new ArrayList(){ {
add("String1");
add("String2");
} };
public User() {
}
public ArrayList<String> getStrings() {
return strings;
}
public void setStrings(ArrayList<String> strings) {
this.strings = strings;
}
}
Main method code:
public class Main {
public static void main(String[] args){
User user = new User();
System.out.println(user.getStrings());
user.getStrings().add("String3");
System.out.println(user.getStrings());
}
}
And the output is:
[String1, String2]
[String1, String2, String3]
I changed the getter to this one:
public ArrayList<String> getStrings() {
return (ArrayList<String>)strings.clone();
}
But the question remains, what are getters for if not for security? And how to write them correctly?
source to share
No, this is not ok, because it breaks encapsulation and therefore the class cannot maintain its own invariants. It's the same with constructors.
But the problem is not that it has getters / setters, but with the code that auto-generates them.
In short: do not blindly use autogenerated accessors if they deal with mutable structures, make defensive copies (or immutable equivalents).
As an aside, I wouldn't have a recipient with a return type ArrayList
, even if it's just a copy. Typically no client business, which list you return, so my recipient would look like this:
public List<String> getStrings() {
return new ArrayList<>(strings);
}
Or using an immutable view:
public List<String> getStrings() {
return Collections.unmodifiableList(strings);
}
Or using the Guava class ImmutableList
:
public List<String> getStrings() {
return ImmutableList.copyOf(strings);
}
There are subtle differences between the three solutions, which can be different. As a general rule, I prefer to return immutable structures because it makes it clear that changes made to the structure will not be reflected, i.e. user.getStrings().add( "X" );
will end with an exception.
Another subtle problem with the code you showed us is the initialization of double brackets. Imagine a class like this:
public class Foo {
private List<String> strings = new ArrayList() {{ add("bar");}};
private Object veryLargeField; //the object stored here consumes a lot of memory
public List<String> getStrings() {
return strings;
}
}
Now, imagine we are doing this:
private class Bar {
private List<String> fooStrings;
public Bar() {
this.fooStrings = new Foo().getStrings();
}
}
How much memory will it Bar
consume (or use the exact term: retain)? Well, that turns out to be quite a lot, because what you do with the double brace initialization creates an anonymous inner class that will hold a reference to its outer class ( Foo
), and thus, when the list is returned, all other fields Foo
will be unusable for garbage collection.
source to share
From my point of view, getters should generally serve two purposes:
- they must guard the implementation details first.
- second, they must provide an easy way to extend (like validation or instrumentation).
If your example violates these principles, it depends on the context:
- If your class needs to own strings, then probably everyone needs to interact with the container object to modify the list, not the list itself. To open a collection (for example, for processing in a method that is expecting a collection), you can use, for example. Collections.unmodifiableList (). If, on the other hand, only a list of strings belongs to a class, then it is not an implementation detail to have a list.
- Using the receiver instead of directly accessing the fields makes it easy to add data exchange, keep track of tools and other things without changing where the field is used.
source to share