Why throw an RecordNotFoundException in your unlock method?

[Sean Keane]

Why would you have your unlock method throw a RecordNotFoundException?

Here is the unlock method from my DB interface:

  // Releases the lock on a record. Cookie must be the cookie
  // returned when the record was locked; otherwise throws SecurityException.
  public void unlock(int recNo, long cookie) throws RecordNotFoundException,
    SecurityException;

Now, when I am unlocking a record I do the following

1. Check if the record is locked - if it is not locked then there is nothing to unlock.
2. Check if the record is locked by the client that is attempting to unlock it.

When I am doing (2) it makes sense to me that I should throw a SecurityException in order to satisfy the requirements from the documentation associated with the unlock method i.e. if a user is attempting to unlock a record that is locked by someone else, then I should throw a SecurityException.

However, when I am doing (1) it really doesn't make sense to me that I should throw a RecordNotFoundException - and I can't see where else in the method I would throw a RecordNotFoundException exception.

My reasoning is simply this. A record may exist, but not be locked, in which case throwing a RecordNotFoundException would not make much sense. It is misleading - the record does exist, it is just not locked.

I guess when I am doing (1), I could do:

1.1 Check if the record exists - if not, throw a RecordNotFoundException
1.2 Assuming the record exists, check that it is locked, if it is not locked then simply return silently without carrying out any action

But what is the real benefit in this second approach?

[Roel De Nijs]

The only method that should throw a RNFE is the lock-method (besides the read and find methods). For update/delete/unlock it makes no sense at all, because if the record does not exist, the lock-method will (and should) throw the RNFE.

[Sean Keane]

Yep, that was my thinking. So Oracle just added the RNFE just to confuse us ?

I'm guessing this is how you implemented yours Roel (i.e. you didn't throw a RNFE from unlock()) so I presume we can assume from this that Oracle don't have some automated test that is expecting a RNFE to be thrown in a certain scenario?

[Roel De Nijs]

Sean Keane wrote:Yep, that was my thinking. So Oracle just added the RNFE just to confuse us ?

It was Sun who added it to confuse you, Oracle just used the same certifications

I presume we can assume from this that Oracle don't have some automated test that is expecting a RNFE to be thrown in a certain scenario?

I don't think that's not tested through an automated test, because it depends on each implementation (and thus could be different each time). Must source requirements (does Data implement required interface, do exceptions have the necessary constructors,... in short all source requirements I tested of in the test case you can find through ScjdFaq ) could be tested with an automated test (like I showed, maybe Oracle is using my test case )

[Sean Keane]

Yes Sun confused me, and as I walked around dazed and confused Oracle stole my wallet

Thanks for the clarification Roel. If you ignored RecordNotFoundException on the unlock method I think it's safe for us all to assume this is OK to do

[Roel De Nijs]

Sean Keane wrote:If you ignored RecordNotFoundException on the unlock method I think it's safe for us all to assume this is OK to do

That might a bit too easy (and maybe it's not ok). But it's certainly ok if you ignore it and document this decision (and that's what i did )

[Roberto Perillo]

Sean, what you can do is, in the update/delete/unlock methods, if the record to be updated/deleted/unlocked was not locked (or if it isn't in the structure that keeps locked records), then an IllegalStateException can be thrown.

[Ixus See]

Sean Keane wrote:Yes Sun confused me, and as I walked around dazed and confused Oracle stole my wallet

Thanks for the clarification Roel. If you ignored RecordNotFoundException on the unlock method I think it's safe for us all to assume this is OK to do

Just wanna confirm this... did you ignore RecordNotFoundException for unlock and pass the exam? I'm planning to ignore it too.. unless I implement if RecordNotFoundException (remove from lockrecords and notify all)

[Sean Keane]

Ixus See wrote:Just wanna confirm this... did you ignore RecordNotFoundException for unlock and pass the exam? I'm planning to ignore it too.. unless I implement if RecordNotFoundException (remove from lockrecords and notify all)

I ignored the exception, but I haven't submitted yet. Others have ignored the exception and passed see here. Once you document that you ignored it along with your reasons then I'm sure you will be fine.

[Ixus See]

oic... I have problem requesting to take the exam and submitting my assignment to Oracle.. I am waiting for their email reply but still unavailable

[Sarah Archer]

I implemented the RNFE on unlock() as it is part of the DB interface I was given and this interface "MUST" be implemented. It is possible that some versions of URLyBird do not declare the exception for unlock() in which case it would then be a matter of personal preference

It also made sense to me to implement this exception. If the record you are trying to unlock() does not exist throw RNFE. If the cookie is wrong then throw SecurityException. If your code is correct then this exception should never be thrown - and neither should the SecurityException - which appears not to be as controversial. The point of the exception is argument validation and Sun recommends that exceptions should be thrown when validating arguments on public interfaces.

That people have not implemented the RNFE and still passed may indicate that it is not an instant fail, but it is by no means certain that points won't be deducted.

To my mind not implementing the RNFE is a very brave decision.

[Ixus See]

Sarah Archer wrote:I implemented the RNFE on unlock() as it is part of the DB interface I was given and this interface "MUST" be implemented.

May I ask if you attempt to a delete an record..

1) first you lock (record still exist)

2) delete reord(record deleted successfully)

3) unlock - throws RNFE, that means record will be lock forever since you 100% will throw RNFE and moreover exception should not be throw under normal condition. (positive flow)

[Sean Keane]

Sarah Archer wrote:It also made sense to me to implement this exception.

How did you handle the scenario when a user was attempting to unlock a record after deleting it?

As Ixus posted. How would it make sense in the context of deleting a record? When a client calls unlock after deleting a record you are guaranteed that the record does not exist. Throwing a RecordNotFoundException in this circumstance would not be appropriate.

So then you have the question of - can I differentiate inside the unlock method between the two cases (A) Where a record was deleted (B) Where a record was not deleted. As far as I can see it the answer is no - you can't differentiate between the two cases.

Because you can't differentiate between the two cases you have to choose to either always throw a RecordNotFoundException or never throw it.

Sarah Archer wrote:That people have not implemented the RNFE and still passed may indicate that it is not an instant fail, but it is by no means certain that points won't be deducted.

To my mind not implementing the RNFE is a very brave decision.

Choosing to never throw the RecordNotFoundException is a safe approach. We can be very confident that you will not lose marks with this approach. If you take a look at this thread here you will see this approach was taken by Roel who passed with 100%.

[Roel De Nijs]

Sarah Archer wrote:I implemented the RNFE on unlock() as it is part of the DB interface I was given and this interface "MUST" be implemented.

You are correct: implementing the given interface is indeed a must requirement. But a method with a throws clause does not have to throw this exception, it may throw the exception.

Sarah Archer wrote:It also made sense to me to implement this exception.

I completely disagree: from lock, update, delete and unlock methods there is just 1 method which should throw the RNFE and the lucky winner is the lock-method. When a record is successfully locked (so no RNFE being thrown), nothing can happen with the record (so certainly not disappear). The only client that could do something with the locked record, is the one that locked it. If that's not the case, you'll have a flaw in your locking mechanism. Therefore it makes no sense at all to throw RNFE from the other methods I mentioned.

Sarah Archer wrote:If the record you are trying to unlock() does not exist throw RNFE. If the cookie is wrong then throw SecurityException. If your code is correct then this exception should never be thrown

Not true. When a record is deleted, you'll get a RNFE being thrown from unlock-method (because the record does not exist anymore). So correct code, but still the exception being thrown.

Sarah Archer wrote:That people have not implemented the RNFE and still passed may indicate that it is not an instant fail, but it is by no means certain that points won't be deducted.

Again not true. I passed with a perfect score and didn't throw the RNFE from unlock-method although the exception was present in the throws-clause of the unlock-method in the given interface. I documented this decision quite heavily in my choices.txt, so maybe if you don't do that, you might lose some points.

Sarah Archer wrote:To my mind not implementing the RNFE is a very brave decision.

I have always been a very brave boy

[Sarah Archer]

Wow - I am starting to see why throwing a RNFE is so contentious

@Ixus I agree that the behaviour you suggest will occur; however, I do not think this a normal flow. Once the record is deleted, then in my view it no longer exists and therefore can no longer be unlocked. In order to be able to call unlock() on a deleted record the record would have to become some kind of zombie.

@Roel I accept that it is possible to write a client that can still function correctly even without unlock() throwing, but I would not consider this good practice. For example imagine a client with a bug calls unlock() for record number 1000 and cookie 0 and record 1000 does not exist and has never existed. What should the behaviour of unlock be

pretend to unlock the record, but do nothing

throw SecurityException even though this is inconsistent with update()

throw RNFE

I know there are many different versions of URLyBird out there, but my instructions state:

"Any methods that throw RecordNotFoundException should do so if a specified record does not exist or is marked as deleted in the database file."

To me it is pretty clear that a RNFE exception should be thrown by unlock.

Roel De Nijs wrote: I passed with a perfect score

Congratulations!

Would love to see your choices.txt though, but alas that can never be...

[Roel De Nijs]

Sarah Archer wrote:@Roel I accept that it is possible to write a client that can still function correctly even without unlock() throwing, but I would not consider this good practice. For example imagine a client with a bug calls unlock() for record number 1000 and cookie 0 and record 1000 does not exist and has never existed. What should the behaviour of unlock be

I assume you mean the bug is that the client calls unlock before calling lock? This is mis-use of the API and will result in throwing an IllegalStateException (my interface does not had a SecurityException), because you can't unlock a record which was not locked. When you try to delete/update a record which was not locked before, you'll get an IllegalStateException too. So the application behaves consistently.

Sarah Archer wrote:"Any methods that throw RecordNotFoundException should do so if a specified record does not exist or is marked as deleted in the database file."

To me it is pretty clear that a RNFE exception should be thrown by unlock.

A should is not a must

Sarah Archer wrote:Congratulations!

Thanks!

Sarah Archer wrote:Would love to see your choices.txt though, but alas that can never be...

That's true (even if we would be married, my choices.txt would be a no-go for you ). It contains of course the explanation why it's completely insane to throw a RNFE from methods like update, delete and unlock if you first have to lock the record. And that I explained in 3 different ways to make sure the accessor would understand my point (being a not native English speaker)