Genius or Madness ... bug or feature
I've inherited some code, no comments, no idea of programmer ability can't talk to either but two seperate programmers
appear to be following 'an interesting'
pattern.
They're basically locking a member variable then changing the reference used to locate the locked object within the synchronized block,
i.e. I exepcetd them to lock on the same object but they don't necessarily.
Object m_state = state1; // where state1, state2 etc are static final objects created elsewhere
in one public member code of this form ...
and in another public member code of this form ...
My problem was that without comments a quick glance the intention of the code wasn't obvious (I suspect its not what the author intended at all)
and led to a whole list of further evils, too many to list and what they actually intended I suspect could probably be achieved in one of many far more simpler options.
Now I'm not sure if the author intended what they have written but would it be reasonable to say this in a code review ...
1) If your not an expert in
java threading never change the reference to the variable you sync on i.e. introduce a new object to lock on without making it very obvious.
2) If you are an expert comment your code to warn others and explain what the intention of the synchronization is i.e. what it does and what it
doesn't.
i.e. my feeling is such code is bordering on evil even if intentional without comments , or are you all doing it ;-).
,
[ October 11, 2006: Message edited by: Jim Yingst ]