M Mehta wrote:Will making the two methods synchronize suffice or is it necessary to use one of synchronized version of map?
I have no idea what you're saying here. However, the general rule is that if any data can be written by one thread at the same time that it's being read or written by another thread, then all access to that data must be synchronized. If any operation needs to occur atomically, you have to synchronize around that operation.
There are higher level constructs in java.util.concurrent that can shield you from the low-level details of syncing, but they all build on the above concepts.
you should make a local, read-only copy of the notes Map in the constructor. If you do that then the class should be thread safe without any further synchronization needs.
Steve Luke wrote:In your specific sample, you only have one piece of persistent data - the notes Map. That Map is iterated over in both the methods but doesn't appear to be modified. The problem is it is passed in from the outside, and so the Map could be modified from an external source. If that happens this class would be unsafe, and there is very little you could do to protect yourself from it.
You should make a local, read-only copy of the notes Map in the constructor. If you do that then the class should be thread safe without any further synchronization needs.
It seems odd that the Map member variable is never modified. We have an instance of an ATM, but calling withdraw() doesn't alter the state of that ATM? There's a bit of design smell there.
@OP: Maybe you have a good reason for doing that, or maybe you're just not showing us the entire class. As the code stands, Steve is correct--since you don't modify the map, as long as you make a copy at construction time, you're safe. However, if there's any way that Map can get modified then you need synchronization. And note that while you're copying the Map for constructing an ATM, if anything else can modify that original Map, then you need synchronization.
Finally, I'll just point out that the "M" in "ATM" stands for "Machine," so your class name has some redundancy.
("Enter your PIN number into the ATM machine" --> "Enter your Personal Identification Number number into the Automatic Teller Machine machine.")