• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

NX: Is my locking methods OK? Pls review.

 
HaoZhe Xu
Ranch Hand
Posts: 222
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
lockedRecords is an instance of HashMap.

Should I use HashTable instead of HashMap? What's the difference?
[Andrew: deleted a couple of methods - see below]
[ January 07, 2004: Message edited by: Andrew Monkhouse ]
 
Philippe Maquet
Bartender
Posts: 1872
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi HaoZhe,
Just a few quick comments before going and sleep :
lock() :
  • You don't check if recNo is valid. An invalid recNo (like -2 ) could lead you to throw a RecordNotFoundException. Same for a "deleted" record BTW.
  • InterruptedException : the way you handle it may be discussed, but what you do is defendable (another solution would be to throw some RuntimeException in that case).
  • The call to notifyAll() is useless.


  • unlock() :
  • A call to notifyAll() is missing after the remove().


  • isLocked() :
  • As in lock, you should test whether recNo is valid or not, or if you don't you should remove the RecordNotFoundException from the throws declaration (what you *may* do BTW, even if the *interface* Data implements declares it).
  • I think you don't need to synchronize the method.
  • I'd have written the 4 last lines in one (but it's just a question of style and personal taste ).


  • Regards,
    Phil.
    PS: HashMap is perfect. You may see HashTable as a sort of old-fashion *and* synchronized version of HashMap. And you don't need built-in synchronization anyway as you synchronize on lockedRecords by yourself (and you need it for wait() / notifyAll() purposes).
    [ January 06, 2004: Message edited by: Philippe Maquet ]
     
    HaoZhe Xu
    Ranch Hand
    Posts: 222
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Thank you very much
     
    Andrew Monkhouse
    author and jackaroo
    Marshal Commander
    Pie
    Posts: 12014
    220
    C++ Firefox Browser IntelliJ IDE Java Mac Oracle
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi HaoZhe
    As you can see, I have deleted a couple of methods from your post. While we do allow people to post small amounts of code, we do not allow people to post their entire solution to one of the major sections of the assignment. You had effectively posted 10% of the assignment.
    Basically we do not want someone to come along and just use your code without having thought about it for themselves first. With the code that remains you can still get comments on the lock() method, and you could always open another post for another method if you like. But no-one can just copy your code - they would have to think about what to do with the other methods.
    I presume you are ensuring that you create one unique instance of the Data class for each connected client.
    You may be interested in this thread for some reasons why you should not use Hashtable.
    Regards, Andrew
     
    • Post Reply
    • Bookmark Topic Watch Topic
    • New Topic