B&S RecordNotFoundException: what exactly does it mean?

[Ryan Kade]

Have a quick question about the spec on RecordNotFoundException. It reads (B&S 2.3.2):

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

The methods it refers to are: read, update, delete, find, lock, and unlock.

My question is, how flexible is this requirement? For example, I throw RecordNotFoundException in the following circumstances:

read
Attempt to read a record marked deleted
*EOFException (if reading past end-of-file)
*IOException (generic)

update
Attempt to update a record past the end of the file
Attempt to update a record marked deleted
*InvalidLockException (custom exception if thread doesn't own lock)
*IOException (generic)

delete
*InvalidLockException (custom exception if thread doesn't own lock)
*IOException (generic)

find
If no matching records are discovered at all
*IOException (generic)

lock
does not get thrown

unlock
If a thread tries to unlock a record that is not locked

* These exceptions are wrapped inside a RecordNotFoundException and unwrapped further up the code.

Three questions:
1. Is wrapping exceptions into a RecordNotFoundException consistent with the spec?

2. My design doesn't call for lock()/unlock() to know anything about the underlying record structure. I really don't want my lock() method to have to verify that a given record is legitimate. That shouldn't be it's job. "You want a lock on record 9000? Fine, go ahead!" It's the other methods' duty to know a record is invalid. But is this consistent with the spec?

3. Similar to #2, my delete does not throw an exception if the user tries to delete a row that has already been deleted, or if it tries to delete a row beyond the end of the file. It just gracefully returns. Is this out of harmony with the spec?

The word "must" is not used, so I'm wondering how strict we need to be in following these guidelines. (Naturally, I can always document it in my choices.txt, but I want to make sure it's not an "automatic" failure or deduction of points).

Any thoughts?
[ March 28, 2008: Message edited by: Ryan Kade ]

[Alex Belisle Turcot]

Originally posted by Ryan Kade:
1. Is wrapping exceptions into a RecordNotFoundException consistent with the spec?

Yes! That is your design choice. I preferred to throw runtime exception instead.
For all I know (opinion), this could result in loosing points in the OO section or general considerations.
You RecordNotFoundException wouldn't represent the real thing..

2. My design doesn't call for lock()/unlock() to know anything about the underlying record structure. I really don't want my lock() method to have to verify that a given record is legitimate. That shouldn't be it's job. "You want a lock on record 9000? Fine, go ahead!" It's the other methods' duty to know a record is invalid. But is this consistent with the spec?

uhm.. I think it is, and maybe not a bad idea if I may
I threw IllegalArgumentException, but that was not mandatory..

3. Similar to #2, my delete does not throw an exception if the user tries to delete a row that has already been deleted, or if it tries to delete a row beyond the end of the file. It just gracefully returns. Is this out of harmony with the spec?

I would be careful here. There's meeting the spec and meeting the spec with points. I think it would meet the spec alright but I'm unsure how the Data class is graded (let alone general considerations).. Why not throw "IllegalArgument" when someone calls delete on a record that is already deleted, or not locked ?
But again, reading the description, you don't have to.

The word "must" is not used, so I'm wondering how strict we need to be in following these guidelines. (Naturally, I can always document it in my choices.txt, but I want to make sure it's not an "automatic" failure or deduction of points).

Many people implemented the exceptions differently, I haven't heard of failure because of it!
[ April 03, 2008: Message edited by: Alex Belisle Turcot ]

[Ryan Kade]

Originally posted by Alex Belisle Turcot:

Yes! That is your design choice. I preferred to throw runtime exception instead.
For all I know (opinion), this could result in loosing points in the OO section or general considerations.
You RecordNotFoundException wouldn't represent the real thing..

This is true, and is a sticking point for me. Wrapping them in RecordNotFoundException wasn't my favorite solution. I suppose runtime would work just as well, I just had preferred that the exceptions (IOException among others) be declared. But perhaps your suggestion is more true to the spirit of what the exceptions are supposed to represent.

Originally posted by Alex Belisle Turcot:

I would be careful here. There's meeting the spec and meeting the spec with points. I think it would meet the spec alright but I'm unsure how the Data class is graded (let alone general considerations).. Why not throw "IllegalArgument" when someone calls delete on a record that is already deleted, or not locked ?
But again, reading the description, you don't have to.

I guess in my opinion, not throwing an exception is better, assuming there is a graceful alternative. For example I can represent "no search results found" as easily in an empty List as I can by throwing an exception, and then no try-catch block is required. But again, the spirit of the spec seems to suggest it needs to be thrown if the user tries to delete an already deleted row.

Thanks for your comments, Alex!

I have one other thought for anyone reading ... about Javadocing the custom exceptions (RecordNotFound, DuplicateKey, etc). Is it poor form to just copy the pertinent text from the official Javadoc for Exception, and use that to document my own methods? With the exception of the purpose for which these custom exceptions are thrown, all the functionality (getMessage(), etc.) is identical.

[Alex Belisle Turcot]

Originally posted by Ryan Kade:
For example I can represent "no search results found" as easily in an empty List as I can by throwing an exception, and then no try-catch block is required. But again, the spirit of the spec seems to suggest it needs to be thrown if the user tries to delete an already deleted row.

I read on the forum many people returned an empty array for findCriteria, and I consider it a nice design choice. I could have gone either way on this one (exception or return empty list).

... about Javadocing the custom exceptions (RecordNotFound, DuplicateKey, etc). Is it poor form to just copy the pertinent text from the official Javadoc for Exception, and use that to document my own methods? With the exception of the purpose for which these custom exceptions are thrown, all the functionality (getMessage(), etc.) is identical.

You most probably don't even need to declare the getMessage method since your inheriting it, so you definitely don't need to document what you don't write

I believe your Exceptions should only contain 2 constructors, which I would
document.

Also, instead of copying javadoc from superclasses, take a look at Automatic Copying of Method Comments:
Javadoc
- Automatically inherit comment to fill in missing text
- Explicitly inherit comment with {@inheritDoc} tag

Alex
[ April 08, 2008: Message edited by: Alex Belisle Turcot ]