This week's book giveaway is in the Agile and Other Processes forum.
We're giving away four copies of The Little Book of Impediments (e-book only) and have Tom Perry on-line!
See this thread for details.
Win a copy of The Little Book of Impediments (e-book only) this week in the Agile and Other Processes forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

update/delete/unlock/isLocked and RecNotFoundEx

 
Alex Sharkoff
Ranch Hand
Posts: 209
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi all,

Let me dive straight into it
I personally think it does not make sense to check whether the record exists in update/delete/unlock/isLocked methods. Or in other words these methods should never throw RecordNotFoundException.


  • why there is no need to check whether the record exists in update/delete methods?

  • Because our contract indicates that the record must be locked prior to being modified (we consider a deletion as some sort of modification). It is during the record locking process that we check whether the record exists.

    The only check that is performed in the update/delete methods is to ensure that the record is locked. If it is not locked by the current client then some kind of runtime exception is thrown. If it is locked then there is 100% guarantee the record exists.
  • why there is no need to check whether the record exists in isLocked methods?

  • Because it can just return false for the non-existent record. It is in the lock method that we check whether the record exist in order to avoid locking the non-existent record.

    It seems that if isLocked was to check whether the record exist it would perform the operation that should be a part of a different method, let's say checkIfRecordExist method. Obviously, this method is not part of the SUN's interface but we could always add it to the implementation. Then if a client needed to check whether the record exist it would just use checkIfRecordExist method (provided that it knew the implementation class) rather than isLocked
  • why there is no need to check whether the record exists in unlock methods?

  • Because we should be able to unlock just deleted record.

    For the record to get deleted it should be locked -> deleted -> unlocked. We could obviously implement unlock method in a such way so that it unlocks the record and then throws RecordNotFoundException. But I think that it does not make sense especially for the client who just deleted the record and now trying to unlock it. The client knows that the record does not exist and it just wants to complete the steps specified by the contract (lock->modify->unlock).




    These are just my thoughts and I'd be very happy to hear your comments.


    [ February 06, 2006: Message edited by: Alex Sharkoff ]
     
    Kevin Conaway
    Ranch Hand
    Posts: 57
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Alex,

    I'll address your questions as best I can.

    I can't speak for your assignment, but mine makes no claim that a record must be locked prior to modification (B&S 2.3)

    Following the principle of single responsibility, a method isLocked() should return false ONLY when a record is not locked. Having it return false when the record does not exist violates this principle. Say I call the method and it returns false, how do I know that the record is not locked or non-existant?

    Furthermore, if you attempt to lock a record and someone else has locked it, why would you want to throw a RuntimeException? Wouldn't it be more prudent to wait until the record is unlocked?

    Personally, I think a RecordNotFoundException can be thrown in two circumstances: 1.) The key actually does not exist in the database or 2.) The key has been marked as deleted.

    In lock()/unlock(), I think you should only be checking for circumstance 1. I can envision a scenario when you would want to lock or unlock a record that is marked as deleted.

    Kevin
     
    Mihai Radulescu
    Ranch Hand
    Posts: 918
    IntelliJ IDE Java Linux
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi, Alex.

    Strange, on my assignment(URLy 1.3.3) neary all the records are throwing RecordNotFoundException.

    Kevin ,

    Wouldn't it be more prudent to wait until the record is unlocked?


    This can be triky, I mean the test method, to check this kind of condition you need a speen lock otherwise a thread can slide between the test statement and its block exactly after the test was pass.


    Regards,
    Mihai
    [ February 06, 2006: Message edited by: Mihai Radulescu ]
     
    Alex Sharkoff
    Ranch Hand
    Posts: 209
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Mihai and Kevin,

    Mihai said:

    Strange, on my assignment(URLy 1.3.3) neary all the records are throwing RecordNotFoundException.

    Yes, my database interface throws RecordNotFoundException from update/delete/unlock/isLocked methods. I meant the actual implementation does not have to throw exceptions declared in the interface that it implements.

    Kevin said:

    Following the principle of single responsibility, a method isLocked() should return false ONLY when a record is not locked. Having it return false when the record does not exist violates this principle. Say I call the method and it returns false, how do I know that the record is not locked or non-existant?

    Good point. I'll modify my isLocked() method to throw RecordNotFoundException for non-existant records.

    Kevin said:

    Furthermore, if you attempt to lock a record and someone else has locked it, why would you want to throw a RuntimeException? Wouldn't it be more prudent to wait until the record is unlocked?

    I do not throw runtime exception from the lock method (the thread waits, as you said). I am throwing runtime exception from delete/update methods when the record is not locked to report unexpected behaviour by the database client. The contract between the client and the database is that if a client wants to modify the record it needs to 1) lock ; 2) modify (delete/update) ; 3) unlock . Therefore if the client breaks the contract agreement he is punished with the runtime exception .

    Kevin said:

    Personally, I think a RecordNotFoundException can be thrown in two circumstances: 1.) The key actually does not exist in the database or 2.) The key has been marked as deleted.

    I agree.

    Kevin said:

    In lock()/unlock(), I think you should only be checking for circumstance 1. I can envision a scenario when you would want to lock or unlock a record that is marked as deleted.

    I'd be always checking for both conditions in order to determine whether the record exist. Yes, you do want to be able to unlock a record that's marked as deleted. That's why I am proposing not to throw RecordNotFoundException from unlock.
    However, why would you want to lock a record that's marked as deleted? If you want to reuse the deleted record in your create method then you can just insert a newly created record into the deleted record's spot and mark it as valid again.

    Kind regards.
    [ February 08, 2006: Message edited by: Alex Sharkoff ]
     
    Yupp Cook
    Ranch Hand
    Posts: 49
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Alex

    I agree to your last statement.
    a) update/delete/lock should check the validity of a record
    b) unlock shouldn't.

    If b) wasn't, lock-delete-unlock would throw a RecordNotFoundException
    If a) wasn't, update/delete/lock on a deleted record would be possible.

    Regards
    Yupp
     
    Alex Sharkoff
    Ranch Hand
    Posts: 209
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Yupp ,

    Yupp said :

    a) update/delete/lock should check the validity of a record

    I am suggesting to take it one step further and check whether the record is locked in update/delete methods rather than whether it exists (only extistent record can be locked, therefore if the record is locked it implies that the record exist)


    Kind regards.
    [ February 08, 2006: Message edited by: Alex Sharkoff ]
     
    Yupp Cook
    Ranch Hand
    Posts: 49
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Of course, Alex. I was just refering to the validity check to point out that unlock shouldn't check if a record is valid. Otherwise how you unlock a deleted rec?

    Regards
    Yupp
     
    • Post Reply
    • Bookmark Topic Watch Topic
    • New Topic