permaculture playing cards*
The moose likes Developer Certification (SCJD/OCMJD) and the fly likes Lock/Unlock Implementation Review Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Murach's Java Servlets and JSP this week in the Servlets forum!
JavaRanch » Java Forums » Certification » Developer Certification (SCJD/OCMJD)
Bookmark "Lock/Unlock Implementation Review" Watch "Lock/Unlock Implementation Review" New topic
Author

Lock/Unlock Implementation Review

Saraswathy Krishnamoorth
Ranch Hand

Joined: Mar 07, 2003
Posts: 73
Please comment on my Lock and unlock implementation. I have a LockManager class that does locking and unlocking.
private HashMap locks;
public synchronized void lock(RemoteData client,int recordNum) {
if (locks!=null) {
recordNumber = new Integer(recordNum);
databaseLock=new Integer(LockManager.DATABASE_LOCK)
//Database Locking
if (recordNum==LockManager.DATABASE_LOCK) {
while(!locks.isEmpty()) {
wait();
}
//Record Locking
while (locks.containsKey(databaseLock) || locks.containsKey(recordNumber) ) {
wait();
}
locks.put(recordNumber,client);
}
public synchronized void unlock(RemoteData client,int recordNum) {
if (locks!=null && locks.size()>0) {
recordNumber = new Integer(recordNum);
if (locks.containsKey(recordNumber) ) {
RemoteData owner=(RemoteData)locks.get(recordNumber)
if (owner==client) {
locks.remove(recordNumber);
notifyAll();
}
}
John Smith
Ranch Hand

Joined: Oct 08, 2001
Posts: 2937

private HashMap locks;

How about private final HashMap locks, for clarity?

public synchronized void lock(RemoteData client,int recordNum) {

Since no operation is performed on client, except for adding it to the collection, a more formal declaration looks better (program to interface/superclass, not to implementation):
public synchronized void lock(Object client,int recordNum)

if (locks!=null) {

The collection object should never be null, so there is no reason to check for it every time. Instantiate it in the constructor.

databaseLock=new Integer(LockManager.DATABASE_LOCK)

Your databaseLock integer is constant, and therefore should be declared as private static final, and created just once.

if (locks!=null && locks.size()>0) {

No reason to check the size, either.


if (locks.containsKey(recordNumber) ) {
RemoteData owner=(RemoteData)locks.get(recordNumber)
if (owner==client) {

How about 1 line instead of 3:
if (locks.get(recordNumber) == client)
Eugene.
[ April 01, 2003: Message edited by: Eugene Kononov ]
Saraswathy Krishnamoorth
Ranch Hand

Joined: Mar 07, 2003
Posts: 73
Thanks a lot Eugene. I will make the neccessary corrections. Just to Clarify..
In my RemoteDataFactoryImpl constructor I instantiate final (Data and LockManger). The getConnection method of RemoteDataFactoryImpl returns a unique RemoteData object. This RemoteData has instance variables Data and LockManger. These instance member variables have a reference to the Data and LockMangaer created in the RemoteDataFactoryImpl. Apart from the corrections you mentioned, is the rest of lock and Unlock implementation correct if I have the above Scenario.
[ April 02, 2003: Message edited by: Saraswathy Krishnamoorth ]
John Smith
Ranch Hand

Joined: Oct 08, 2001
Posts: 2937

In my RemoteDataFactoryImpl constructor I instantiate final (Data and LockManger). The getConnection method of RemoteDataFactoryImpl returns a unique RemoteData object. This RemoteData has instance variables Data and LockManger. These instance member variables have a reference to the Data and LockMangaer created in the RemoteDataFactoryImpl. Apart from the corrections you mentioned, is the rest of lock and Unlock implementation correct if I have the above Scenario.

Perfect!
Eugene.
Saraswathy Krishnamoorth
Ranch Hand

Joined: Mar 07, 2003
Posts: 73
Thank you very much Eugene.
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: Lock/Unlock Implementation Review
 
Similar Threads
make server threadsafe. How??? please help!
How to understand "Lock the database"
Database Locks
lock/unlock
lock/unlock review