Without a doubt, item 39: Make defensive copies when needed.
I had written a class that held a list of things, and on that class I'd written a getAllTheThings() type method that returned the whole List. It hadn't occurred to me at the time that all I was doing was dishing out references to that single internal List and as soon as one of those client processed decided to start hoking about with the List chaos ensued. All the other clients that held a reference to the same List started exhibiting unexpected behaviour, it took me hours to figure out what I'd done.
Tim Cooke wrote:Without a doubt, item 39: Make defensive copies when needed.
I had to go back and read this again. It's something I'm not doing and I'm trying to understand when I should be doing it. The last paragraph says "if a class has mutable components that it gets from or returns to it's clients". That would seem to mean just about any parameters or return values, but I'm sure I'm not understanding this correctly and I would end up overusing this technique when it's not needed.
Would you care to take a shot at explaining this in a way that my feeble brain might be able to grok?
"The good news about computers is that they do what you tell them to do. The bad news is that they do what you tell them to do." -- Ted Nelson
Defensive copies is a good one. As an example, suppose you have a cache of forums like we do here. Many places in the code get a reference to that same list. Now suppose one method was written by a new developer who wants to display a view with only five forums in it. So he/she calls remove() a bunch of times on that list object. The problem is that everyone has a reference to the same list object and the cache is now corrupt. A defensive copy would prevent that by making it so everyone doesn't have the same list object.
Jeanne's example is the exact one that I encountered. I was writing some tests and decided to write a fake DAO backed by a List so I didn't have to get hooked up to Oracle. My DAO just dished out a reference to the same list everywhere so any changes the client of the DAO made to the List was observed by everyone. Mayhem ensued.
In this case it would only fail in my tests as the real DAO constructed new objects each time but it was a harsh reminder of how Java passes variables around.
So if I understand correctly, it really comes down to the question of "is there a possibility of more than one thread accessing (changing) this object". If in doubt, make a defensive copy.
What about static synchronized methods? This code came to mind:
Should I be making a defensive copy of that dateString parameter?
"The good news about computers is that they do what you tell them to do. The bad news is that they do what you tell them to do." -- Ted Nelson
It's not just multiple threads (although having immutable objects does help with that).
The way I look at it is this: an object should be responsible for maintaining its own state. All changes to that state should go through methods belonging to that object - I have to ask you to change your own state rather than changing it for you.
As soon as an object gives a reference to part of its internal state that is mutable to anyone else, then it has lost control. It no longer has any say in whether that state changes or not. Which means you, as the programmer, can't rely on changes only happening where you expect them to.
In your example, though: Strings are immutable. So a defensive copy there is pointless.
Forget this weirdo. You guys wanna see something really neat? I just have to take off my shoe .... (hint: it's a tiny ad)
a bit of art, as a gift, that will fit in a stocking