9

Possible Duplicate:
Avoid synchronized(this) in Java?

What is the difference between the two pieces of code ? What are advantages and disadvantages of each?

1)

public class Example {
    private int value = 0;

    public int getNextValue() {
        synchronized (this) {
            return value++;
        }
    }
}

2)

public class Example {
    private final Object lock = new Object();
    private int value = 0;

    public int getNextValue() {
        synchronized (lock) {
            return value++;
        }
    }
}
Oleksandr Pyrohov
  • 14,685
  • 6
  • 61
  • 90
AllTooSir
  • 48,828
  • 16
  • 130
  • 164
  • 2
    The 2nd approach is almost always better (AFAIK) -- but why? What negative impacts does the visibility of `this` (just "the object" from the outside) have? How could "bad code" cause interference with the synchronization target? –  Jun 23 '12 at 07:00
  • 1
    Are you aware that `pubilc synchronized int getNextValue(){...}` is exactly the same as `public int getNextValue(){synchronized(this){...}}` ? – Miquel Jun 23 '12 at 09:47

4 Answers4

8

The main reason why I would choose the 2nd approach is that I do not control what the clients do with the instances of my class.

If, for some reason, somebody decides to use an instance of my class as a lock, they will interfere with the synchronization logic within my class:

class ClientCode {
    Example exampleInstance;

    void someMethod() {
        synchronized (exampleInstance) {
            //...
        }
    }
}

If, within my Example class, I'm using a lock that no one else can see, they cannot interfere with my logic and introduce an arbitrary mutex like in the above scenario.

To sum up, this is just an application of the information hiding principle.

Costi Ciudatu
  • 37,042
  • 7
  • 56
  • 92
3

I would prefer the second option if I need to execute two different tasks simultaneously which are independent of each other.

e.g.:

public class Example {
    private int value = 0;
    private int new_value = 0;
    private final Object lock1 = new Object();
    private final Object lock2 = new Object();

    public int getNextValue() {
        synchronized (lock1) {
            return value++;
        }
    }

    public int getNextNewValue() {
        synchronized (lock2) {              
            return new_value++;
        }
    }
}
John Kugelman
  • 349,597
  • 67
  • 533
  • 578
Kumar Vivek Mitra
  • 33,294
  • 6
  • 48
  • 75
0

I would say the second method is better. Consider the following situation:

public class Abc{

    private int someVariable;

    public class Xyz {
        //some method,synchronize on this
    }

        //some method, and again synchronize on this


}

In this situation this is not the same in the two methods. One is a method of the inner class. Hence, it is better to use a common object for synchronization. E.g., synchronized (someVariable).

Kazekage Gaara
  • 14,972
  • 14
  • 61
  • 108
0

I think it really depends on the situation. Lets say your class is a subclass and the super class has a method that has synchronization. And lets say you are working with the same data set and want to maintain integrity within your method as well. Then definitely approach 1 is what you should be using.

Otherwise second approach would work better based on what Costi mentioned

Vidyanand
  • 967
  • 8
  • 14
  • I agree with you but in this case I would say the superclass is not quite properly designed; it could provide a `protected` lock that I can use in a subclass without making the whole synchronization logic publicly available. – Costi Ciudatu Jun 23 '12 at 10:26
  • 1
    @CostiCiudatu Completely Agree with you on that. This is only in cases where you inherit some classes from someone else's codebase. This is probably rare. – Vidyanand Jun 23 '12 at 10:33
  • +1 for the good quality sarcasm -- `This is probably rare` :). My point was just to emphasize that this approach is *not* recommended when you're able to choose, but just the only available way in some scenarios. – Costi Ciudatu Jun 23 '12 at 11:05