In java, return value within synchronized block seems like bad style. Does it really matter?

Question

I have a Collections.synchronizedList of WeakReference, _components;

I wrote something like the following, expecting the complier to complain:

public boolean addComponent2(Component e) {
    synchronized (_components) {
        return _components.add(new WeakReference<Component>(e));
    }        
}

But the compiler is perfectly satisfied. Note that List.add() returns TRUE. So ok, any exit from a synchronized block releases the lock, but doesn't this LOOK strange? It's kind of like a "hole" in the block, similar to using return in a loop.

Would you be happy maintaining code like this?

Panlogism

Answer 1

It's absolutely fine - as is returning from a loop, or from a try block which has an appropriate finally block. You just need to be aware of the semantics, at which point it makes perfect sense.

It's certainly simpler code than introducing a local variable for the sake of it:

// Ick - method body is now more complicated, with no benefit
public boolean addComponent2(Component e) {
    boolean ret;
    synchronized (_components) {
        ret = _components.add(new WeakReference<Component>(e));
    }
    return ret;
}

Paronychia

While I usually return from inside the block too (actually, I don't find myself using primitive synch blocks anymore much anyway, but still) there may be some benefit in doing this from the start. If you only have the synch block, and later some extra code has to be added but that code doesn't require synchronization, then you already have it broken out. If it isn't broken out, future devs in a hurry may just add the extra code inside the synch block since it's easier, which may tie up the monitor unnecessarily. So I'd still return form inside, but there is some minor benefit for the future. – Preciosity

@corsiKa: There's only benefit for the future if that change is required. If it's not required, then it makes reading the code that little bit harder the whole time, IMO. – Paronychia

Answer 2

There is nothing wrong with returning inside a synchronized block. The lock will be released correctly.

Scrivings