URLyBird 1.2.1 - SecurityException

[Philippe Maquet]

Documenting my suncertify.db package, I now hesitate about SecurityException thrown by a few methods in Data.
At first, as SecurityException exists in java.lang, I thought that the declared SecurityException was that one. But java.lang.SecurityException is a RuntimeException, and I read this in the javadoc tool documentation :

By convention, unchecked exceptions should not be included in a throws clause. (Including them is considered to be poor programming practice. The compiler treats them as comments, and does no checking on them.)

So it could make sense to define a checked SecurityException in the suncertify.db package.
What do you think ?
Best,
Phil.

[Andrew Monkhouse]

Hi Philippe,
I guess the question could be rephrased as: should the ScurityException thrown by the Data class be a checked exception?
And I am not sure whether it should be or not. If a programmer is following the correct way of working (lock the record, verify, modify, unlock) then the security exception will not be thrown. So having it unchecked will mean that there is less coding for the good programmer. But the bad programmer will have their application come crashing down the first time they try and skip steps (such as trying to modify without locking first).
Is this a bad thing?
If the JavaDoc you write for the Data class and/or the interface clearly state what is meant to happen in a good application, and the consequences that will occur if not, then the programmer who gets caught by this has only themselves to blame.
By the way, my reading on that section you quoted does not discourage me from throwing unchecked exceptions. My reading is that:

Dont throw an unchecked exception where you should have thrown a checked exception, and expect that documenting it will make up for the bad coding

Dont expect that documenting that you are throwing an unchecked exception will magically transform it into a checked exception.

Sun do document unchecked exceptions in their APIs (Integer.parseInt()), so it is not the documenting in itself that is the bad thing - it is the assumptions that sometimes go with it.
Regards, Andrew

[Philippe Maquet]

Hi Andrew,
Thank you for your reply. BTW (1), you're still the first on the ball as I can see , and BTW (2), congratulations for your recent promotion as a bartender ! Your participation in this forum is so amazing in quality as in quantity that I often wondered when you'd become moderator here. Now I finally got an answer to that obsessional question... Thanks again.
Back to this topic.

I guess the question could be rephrased as: should the SecurityException thrown by the Data class be a checked exception?

Well yes but better as : is the SecurityException mentioned in the throws clause of a few methods of the DBAccess interface :

the well-knowned unchecked java.lang.SecurityException ?

or a checked suncertify.db.SecurityException of our own ?

I mentioned JavaDoc because my quote comes from its doc, but my question has nothing to do with documentation.
My tests show that if I define a suncertify.db.SecurityException, it will have precedence in the suncertify.db package over the java.lang.SecurityException. It means that it's possible to make SecurityException a checked one without altering the method signatures (in the "throws" clause, SecurityException is declared with no full path).
If I take the given quote back :

By convention, unchecked exceptions should not be included in a throws clause. (Including them is considered to be poor programming practice. The compiler treats them as comments, and does no checking on them.)

I understand that if I choose [1] above, it means that Sun defined its DBAccess interface in a "poor programming" fashion : as java.lang.SecurityException is unchecked, it should not be part of the method throws clause (even if it could be good documenting practice to mention it in the doc with a @throws tag.
In other words, if java.lang.SecurityException did not exist, I wouldn't have any question : [2] is better of course. I am just hesitating because "SecurityException" is a well-known existing exception.
I hope this post was more clear
Best,
Phil.
[ August 25, 2003: Message edited by: Philippe Maquet ]

[Vlad Rabkin]

Hi Phil,
I thought pretty much about this problem also.
A agree with Andrew:

I guess the question could be rephrased as: should the SecurityException thrown by the Data class be a checked exception?
...
But the bad programmer will have their application come crashing down the first time they try and skip steps (such as trying to modify without locking first).

First, instructions are very clear:

Any unimplemented exceptions in this interface must all be created as member classes of the suncertify.db package

SecurityException IS an implemented exception by Sun.
It is a bad practice to have the same names of classes even if they are in different packages.
So, I don't think it is a good idea to implement own suncertify.db.SecurityException.
The only question is if we should catch it or not. As Andrew said, in my design, it is impossible to be thrown. So, SecurityException would mean an implementation error. So, we don't have to catch it. Somebody could say that Security exception could be thrown if an authorized request is made (e.g. hucker attack). In that case it would be probably reasonable to catch. Specificication of our assignement doesn't says anything about security issue.
So, I have decided not to catch it.
Regards,
Vlad

[Philippe Maquet]

Hi Vlad,

SecurityException IS an implemented exception by Sun.
It is a bad practice to have the same names of classes even if they are in different packages.

Good point, thanks. But as it's also bad practice to put an unchecked exception in the throws clause of a method, anyway, I have to choose among two bad programing practices... Thank you SUN !
But yes, I think I'll choose the same bad practice as you did.
Best,
Phil.

[Vlad Rabkin]

Hi Phil,

But as it's also bad practice to put an unchecked exception in the throws clause of a method, anyway

I think the only purpose of it is to instruct us to throw SecurityException in appropriate cases.
Vlad

[Roel De Meester]

I'm working on 1.1.2 which is fairly similar
I'm going to implement SecurityException as checked exception within my project. Why?
From my instructions i read :

Any unimplemented exceptions in this interface must all be created as member classes of the suncertify.db package

So is this Security unimplemented or already implemented as java.lang.SecurityException? I think it is NOT the java.lang.SecurityException because:
1) From the javadoc of java.lang.SecurityException i read :

Thrown by the security manager to indicate a security violation.

We are NOT a security manager!
2) From my instructions i read :

You must not require the installation of a security manager.

We are not a security manager and are not installing one!
3) From javadoc tool (as already mentioned by philippe).

By convention, unchecked exceptions should not be included in a throws clause. (Including them is considered to be poor programming practice. The compiler treats them as comments, and does no checking on them.)

[ January 17, 2004: Message edited by: Roel De Meester ]
[ January 17, 2004: Message edited by: Roel De Meester ]
[ January 17, 2004: Message edited by: Roel De Meester ]
[ January 21, 2004: Message edited by: Roel De Meester ]

[Philippe Maquet]

Hi Roel,
Aw, an old thread of mine coming back to life !
Welcome to JavaRanch and this forum !
Roel, I agree with all your arguments above (1-3).
BTW, in this whole thread, the only clear argument *against* implementing SecurityException as a checked suncertify.db.SecurityException of our own, was the one given by vlad :

It is a bad practice to have the same names of classes even if they are in different packages.

I was not 100% convinced in August as you may guess from what I wrote above : indeed, one of the purposes of Java packages is to help in resolving names conflicts. It's not "just by chance" that you're encouraged to base-name your packages with your company's reversed URL address : "com.myCompany.myPackage.*".
Since my previous post in this thread, I found out such a naming conflict within SUN's APIs, solved thanks to packaging. Unfortunately, I cannot remember which one. If anybody reading this has one in mind, thanks in advance to post it here !
Best,
Phil.

[Derek Canaan]

Hi Phil,

I found out such a naming conflict within SUN's APIs, solved thanks to packaging. Unfortunately, I cannot remember which one

Is java.util.Date and java.sql.Date, an example of what you are referring to?
I found a more 'relevant' example:
java.rmi.UnknownHostException and java.net.UnknownHostException
Both have same ancestor, java.io.IOException, a checked exception.
rgds,
derek
[ January 18, 2004: Message edited by: Derek Canaan ]

[Jacques Bosch]

Really does sound logical to define our own suncertify.db.SecurityException in stead of using java.lang.SecurityException.
Another question:
I've also defined a RuntimeIOException that extends RuntimeException and that I use as a wrapper for the IOExceptions that might be thrown in my data class. I.e. RuntimeIOException is not in my thrown clause.
Should I still add it to the java doc comment with an @throw tag? Or should I just specify that it might be thrown in the method description?
Thanx.

[Philippe Maquet]

Hi Jacques,

I.e. RuntimeIOException is not in my thrown clause.

Yes, and it's OK. The compiler would accept you to add it to the throw clause, but it's bad practice.

Should I still add it to the java doc comment with an @throw tag? Or should I just specify that it might be thrown in the method description?

Yes, you should IMO. In fact, I would even do both (@throws tag + method description), + a comment in the general description of the Data class (as that exception may be thrown by most of the methods). As a RuntimeException is not checked by the compiler, I think the less you take the risk in your doc that the user of your class will not notice it, the better it is.
Best,
Phil.
PS: Thanks Derek for the two examples which demonstrate what I wrote above. Quite difficult to retrieve in they don't immediately come to mind !

[Jacques Bosch]

Sorry Phil. Only saw your post today.
Thank you. That was my feeling too.
PS I'm 3/4 the way through the book you sent me. (Thanx again!)

[Derek Canaan]

Hi,

[Phil]: But as it's also bad practice to put an unchecked exception in the throws clause of a method, anyway,

Phil, why do you say that? The SUN API document for Integer.parseInt(String) method has a throw clause and the parseInt(String) method signature *explicitly* throws a Runtime exception (NumberFormatException).
For the Data class that implements the Sun provided DBAccess interface, i would document the same as what Integer.parseInt(String) method does. Since Data is implemented by the candidate, we have a free hand. But what about the DBAccess? Would you change what is originally given to us in your java documentation?
The latter question is discussed in this thread. Please reply there as it's more specific to the topic heading.

thanks,
derek
[ January 20, 2004: Message edited by: Derek Canaan ]

[Roel De Meester]

This is no longer relevant in this thread but i'll post it anyway.
in my assignment. I have a DB interface implemented by Data.java.
The javadoc of DB interface consists only of the documentation giving in the snippet of the instructions.html, on which i added some CONTRACT statements.

/*
 * DB.java 1.0 15-dec-2003
 * 
 * COPYLEFT
 */
package suncertify.db;
/**
 * The required interface for the database layer in the SCJD 1.1.2 assignment.
 * 
 * @author Roel De Meester
 * @author SUN
 *  
 */
public interface DB {
/**
 * Reads a record from the file.
 * <p> 
 * <B>contract: </b> Strings are always returned with ending whitespace removed!<br>
 * <B>contract: </b> Unchecked "wrapped" exceptions can be thrown in case of read problems<br>
 * </P>
 * 
 * @param recNo
 *            the index number of the record to read
 * @return an array where each element is a record value,
 * @throws RecordNotFoundException
 *             if the requested record is not in the database file
 */
public String[] read(int recNo) throws RecordNotFoundException;
/**
 * Modifies the fields of a record.
 * <p> <B>contract: </b> Unchecked "wrapped" exceptions can be thrown in case of write problems<br></P>
 * 
 * @param recNo
 *            the index number of the record to modify
 * @param data
 *            The new value for field n appears in data[n]
 * @param lockCookie
 *            the cookie to use for this operation
 * @throws RecordNotFoundException
 *             if the requested record is not in the database file
 * @throws SecurityException
 *             if the record is locked with a cookie other than lockCookie.
 */
public void update(int recNo, String[] data, long lockCookie)
throws RecordNotFoundException, SecurityException;
/**
 * Deletes a record, making the record number and associated disk storage
 * available for reuse.
 * 
 * @param recNo
 *            the index number of the record to delete
 * @param lockCookie
 *            the cookie to use for this operation
 * @throws RecordNotFoundException
 *             if the requested record is not in the database file
 * @throws SecurityException
 *             if the record is locked with a cookie other than lockCookie.
 */
public void delete(int recNo, long lockCookie)
throws RecordNotFoundException, SecurityException;
/**
 * Finds all records based on given criteria. A null value in criteria[n]
 * matches any field value. A non-null value in criteria[n] matches any
 * field value that <b>begins</b> with criteria[n]. (For example, "Fred"
 * matches "Fred" or "Freddy".)
 * 
 * @param criteria
 *            the criteria used for finding a record: Field n in the
 *            database file is described by criteria[n]
 * @return an array of record numbers that match the specified criteria. If
 *         no results where found an array of length 0 is returned.
 */
public int[] find(String[] criteria);
/**
 * Creates a new record in the database (possibly reusing a deleted entry).
 * Inserts the given data, and returns the record number of the new record.
 * 
 * @param data
 *            the new data to be entered in the database
 * @return the record number of the new record
 * @throws DuplicateKeyException
 *             if a record in the database already exists with the given
 *             key
 */
public int create(String[] data) throws DuplicateKeyException;
/**
 * Locks a record so that it can only be updated or deleted by this client.
 * If the specified record is already locked by a different client, the
 * current thread gives up the CPU and consumes no CPU cycles until the
 * record is unlocked.
 * 
 * @param recNo
 *            the index number of the record to lock
 * @return cookie that must be used when the record is unlocked, updated,
 *         or deleted.
 * @throws RecordNotFoundException
 *             if the requested record is not in the database file
 */
public long lock(int recNo) throws RecordNotFoundException;
/**
 * Releases the lock on a record. Cookie must be the cookie returned when
 * the record was locked; otherwise throws SecurityException.
 * 
 * @param recNo
 *            the index number of the record to lock
 * @param cookie returned when the record was locked
 * @throws RecordNotFoundException
 *             if the requested record is not in the database file
 * @throws SecurityException
 *             if the cookie used to unlock this record in not the same as
 *             the one used to lock the record
 */
public void unlock(int recNo, long cookie)
throws RecordNotFoundException, SecurityException;

}

while the in Data.java javadoc i only ADD some extra info like @throws DBAccessException, and for other info a refer to the DB javadoc using the @see tag.

    /**
     * UNSYNCHRONIZED. All db-access is done by read() which is already
     * synchronized
     * 
     * @see suncertify.db.DB#find(java.lang.String[])
     * @throws DataAccessException.
     *             This unchecked exception wraps an IOException in case of
     *             read problems
     */
    public int[] find(String[] criteria) {
...
    }

while writing this.. i'm thinking that it may be better to mover the CONTRACTS to Data also.. i have to think it over.
Does anyone know the 'correct' way to add contracts to a javadoc?
[ January 21, 2004: Message edited by: Roel De Meester ]
[ January 21, 2004: Message edited by: Roel De Meester ]

[George Marinkovich]

Hi Roel,

    /**     
     [B]* {@inheritDoc}[/B]     
     * UNSYNCHRONIZED. All db-access is done by read() which is already
     * synchronized     
     */
    public int[] find(String[] criteria) {
        ...
    }

You might want to try {@inheritDoc} rather than @see. It will give you the entire find method comment from the interface DB that the Data class implements. It will result in production of javadoc documentation that looks like it was based on something like the following:

/** 
 * Finds all records based on given criteria. A null value in criteria[n]
 * matches any field value. A non-null value in criteria[n] matches any 
 * field value that <b>begins</b> with criteria[n]. (For example, "Fred
 * matches "Fred" or "Freddy".) 
 *
[B] * UNSYNCHRONIZED. All db-access is done by read() which is already
 * synchronized     
 *[/B]
 * @param criteria 
 *            the criteria used for finding a record: Field n in the
 *            database file is described by criteria[n] 
 * @return an array of record numbers that match the specified criteria. If
 *         no results where found an array of length 0 is returned. 
[B] * @throws DataAccessException.     
 *             This unchecked exception wraps an IOException in case of
 *             read problems[/B]
 */

Hope this helps,
George
[ January 21, 2004: Message edited by: George Marinkovich ]