Final check before submission

[Michael Tu]

Dear friends,
Have completed the assignment with help readily available from this forum. Would like to have your comments on my design:
I have three packages, i.e. suncertify.db, suncertify.client and suncertify.server.
In suncertify.db, I have following classes and interfaces:
1. public interface DataInterface: All basic database operations are defined here, including criteriaFind, lock and unlock. Generic Exception is thrown in all methods.
2. public class Data implements DataInterface: The original Data class is modified to implement DataInterface. criteriaFind method is implemented while lock and unlock remain empty.
3. public class criteriaParser: This is a utility to parse generic criteria string.
4. public class LockManager: LockManager is implemented as Singleton. It keeps a reference to Data. lock & unlock are only implemented for network mode. Client ID uses the unique connection object (For each connection, a RemoteData object is instantiated).
5. DataInfo, FieldInfo and DatabaseException remain unchanged.

In suncertify.server, there are:
6. public interface RemoteDataInterface extends DataInterface, Remote: Basic database methods are exported here again. DatabaseException and RemoteException are thrown for all methods.
7. public class RemoteData extends UnicastRemoteobject implements RemoteDataInterface, Unreferenced: All methods defined in RemoteDataInterface and the unreferenced method are implemented. It keeps a reference to Data class and a reference LockManager. Only one instance of Data class and one instance of LockManager are instantiated and shared by all connections.
8. public interface FBNServerInterface extends Remote: Only one method getRemoteDataInstance() is defined. The method just creates and returns a RemoteData to each caller.
9. public class FBNServer extends UnicastRemoteObject implements FBNServerInterface: This is an implementation of FBN server. It is actually a connection factory to create RemoteData object for each connection. Only one instance of Data object is instantiated and passed to all connections. The server binds itself to a default or given port.

In suncertify.client package,
10. public class ClientData: This is a wrapper class for local or remote database services. It automatically connects to local or remote database based on the database address argument upon instantiation. Again, all basic database operations are implemented.
11. public class DataService: This is a data Facade which only implements high-level business transactions (e.g. flight query and booking). It also implements Observable and serves as the model class in the Model-View pattern (with following FBNMain as the view class). It keeps all updated flight data and notifies its observer any changes.
12. public class FBNMain extends JFrame, Observer: This is the UI for FBN system. It registers itself as a view of DataService.

About UI:
Have a menubar, a toolbar. Basically, a BorderLayout style is employed. There are three panels: Search flight panel, flight info panel and book flight panel. The north is the Search flight panel which contains Orgin and Destination ComboBoxes and a Search button. The center is the flight info panel which is merely a JTable showing flights. The book panel located south with a Flight No ComboBox, editable Seat Num comboBox and a Book button.

Would be very appreciated if I can have you guy's comments. Pls...

Michael

[Peter den Haan]

Originally posted by Michael Tu:
1. public interface DataInterface: All basic database operations are defined here, including criteriaFind, lock and unlock. Generic Exception is thrown in all methods.

Yikes. Can you justify that last point? Personally, I've come to loathe APIs that throw a completely amorphous Exception at you. You can make a case for throwing IOException, but Exception...?

4. public class LockManager: LockManager is implemented as Singleton. It keeps a reference to Data.

Please revisit your Singleton assumption; it strikes me as a serious architectural mistake. Apart from the FBN assignment, how many database-driven applications do you know that use exactly one database table? In general, Singleton is the most abused design pattern around. Never use it unless there is a solid, fundamental reason why you can have only one instance of a class. The fact that you happen to need just one in a particular application is neither solid nor fundamental.
What does the LockManager need the reference to Data for?

6. public interface RemoteDataInterface extends DataInterface, Remote: Basic database methods are exported here again. DatabaseException and RemoteException are thrown for all methods.

Why bother? Just have

interface RemoteDataInterface
extends DataInterface, Remote {
   // that's all folks
}

10. public class ClientData: This is a wrapper class for local or remote database services. It automatically connects to local or remote database based on the database address argument upon instantiation. Again, all basic database operations are implemented.

Junk this class, or turn it into a factory. It certainly shouldn't implement DataInterface. All you need is a place to instantiate a DataInterface object that is either Data, or a RemoteData stub. The rest of the application can take this object and work with that without worrying whether it's a local or remote implementation.
You can use a factory (ClientDataFactory?) to create the appropriate object based on the database location. Or you can make it a simple static factory method in FBNMain called from your main function. Your call.
HTH,
- Peter

[Michael Tu]

Peter,
Thanks for your valuable comments. My explainations are as follows:

You can make a case for throwing IOException, but Exception...?

Yes, I also dislike this generic Exception. However, seems there is no choice here: DataInterface is used by both local and remote mode. The exceptions involved will comprise at least IOExcetpion, DatabaseException and RemoteException. If you list all of them, the method definitions will be quite long, which I also don't like. A generic one looks more concise. Actually, I do throw specific exceptions in the implementations of local and remote database methods. For example:
modify method throws DatabaseException in Data class; throws RemoteException and DatabaseException in RemoteData class.

Apart from the FBN assignment, how many database-driven applications do you know that use exactly one database table? In general, Singleton is the most abused design pattern around. Never use it unless there is a solid, fundamental reason why you can have only one instance of a class.

Initially, I also used one instance of LockManager for all connections, i.e I instantiate an instance of it in FBNServer and pass it to all RemoteData objects. I changed it to Singleton because I want each RemoteData gets a reference to LockManager within itself when it is initialized instead of receiving the reference from FBNServer. I don't quite understand what you mean by mentioning 'one database table'? What are the implications here if database has multiple tables? Will it causes difference between Singleton and one-instance approach? Please elaborate.
The LockManager keeps a reference to Data for some basic database info, such as total record count.

Why bother? Just have
code:
interface RemoteDataInterfaceextends DataInterface, Remote { // that's all folks}

No need to export methods here? I may have a try.

All you need is a place to instantiate a DataInterface object that is either Data, or a RemoteData stub.

This is what I do.
Do appreciate your comments and please explain more on the points I am not very clear.
Michael

[Peter den Haan]

Originally posted by Michael Tu:
[...] If you list all of them, the method definitions will be quite long, which I also don't like. A generic one looks more concise.

Laziness of the right kind is usually a good attribute in designers and coders. This is not the right kind. I can say a few things about this.

You're throwing away all information about the types of problems that can occur and are sabotaging effective exception handling and error recovery on the client end.

Best practice for handling exception bloat is not to change the method signature to throw Exception, but to use either inheritance or wrappers. You can use inheritance by making all your exceptions a subclass of some application-specific base exception, and then throw that base exception in the method signature. IOException is an example of this. Where this is not possible, for example because you don't have direct control over the exceptions being thrown, you can introduce an application-specific exception wrapper. Take for example, SQLException, which essentially can wrap all kinds of error conditions in the database.

But you don't need to do that here. Three exceptions is not much! Besides, they are really just two, corresponding to the two broad classes of problems you can encounter: logical database problems (DatabaseException), and I/O problems (IOException).

If I'd do the assignment again, I would throw IOException rather than RemoteException. Why? Decoupling. By throwing IOException, DataInterface does not make any assumptions about the technology that is used to connect to the server, i.e. sockets or RMI. Please note that RemoteException is a subclass of IOException.

[...] I don't quite understand what you mean by mentioning 'one database table'? [...]

Your instructions probably told you to code for reuse. The Data class corresponds to a single, local database table. Now you've built a perfectly generic database server, but your architecture restricts it to a single LockManager, and therefore to a single Data instance. Which immediately makes it unusable to the 99.9% of the applications out there that actually need to have more than one database table. You've just ruined your reusability. If the gains in simplicity were huge, it'd be justifiable perhaps, but they aren't.

The LockManager keeps a reference to Data for some basic database info, such as total record count.

Does this class need to be aware of anything other than the locked record numbers and their respective clients?

No need to export methods here? I may have a try.

If you don't need to narrow exceptions, the RemoteDataInterface is fully defined by the two interfaces it extends.

This is what I do.

So where does the ClientData wrapper class come in?
- Peter

[Michael Tu]

Peter,
You have very good points. I am seriously considering to (pls comments if anything not correct.):
1) Change the method signatures in DataInterface to throw IOException and DatabaseException.
2) Not implement the LockManager as Singleton. One instance of it will be created by FBNServer and passed to all connections. The RemoteData will have a constructor like:
public RemoteData(Data db, LockManager lm)
and in FBNServer, there will be sth like:

...
Data db = null;
LockManager lm = null;
//constructor
public FBNServer(String dbname) {
    db = Data(dbname);
    lm = LockManager();
}
//implementation of the interface method
public RemoteDataInterface getRemoteDataInstance() {
    return RemoteData(db, lm);
}

3) take out all method definitions in RemoteDataInterface. I've tried it and works.

However, the following points need more clarifications:

Does this class need to be aware of anything other than the locked record numbers and their respective clients?

Don't we need record count for validation checking of a record number? What about if a record number out of range, throw exception?

So where does the ClientData wrapper class come in?

I do it in ClientData class. Pls see my code:

 public class ClientData {
    public static final int LOCAL_MODE = 1;
    public static final int REMOTE_MODE = 2;
    private static String SERVER_NAME = "fbn_server";
    private int dbmode;
    private String source;
    private DataInterface data = null;
    /**
     * This constructor connects caller to local or remote database 
     * automatically based on the supplied database address. 
     *
     * @param address The address of database service. In local mode, it is
     *      the full path of db.db; in network mode, it is the URL address of
     *      the RMI server.
     * @exception java.lang.Exception Thrown when database or network 
     *      connection fails.
     */
    public ClientData(String address) throws Exception {
        source = address;
        dbmode = LOCAL_MODE;
        if (source.startsWith("rmi://")) {
            dbmode = REMOTE_MODE;
        } else if (source.startsWith("//")) {
            dbmode = REMOTE_MODE;
            source = "rmi:" + source;
        } 
        try {
            if (dbmode == LOCAL_MODE) {
                data = (DataInterface) new Data(source);
            } else if (dbmode == REMOTE_MODE) {
                FBNServerInterface srv = (FBNServerInterface)Naming.lookup(
                                          source + "/" + SERVER_NAME);
                data = (DataInterface)srv.getRemoteDataInstance();
            }
        } catch (Exception e) {...}
    public FieldInfo[] getFieldInfo() throws Exception {
        return data.getFieldInfo();
    }
    
    public int getRecordCount() throws Exception {
        return data.getRecordCount();
    }
    ...
}
 

The ClientData class is implemented merely for this requirement:

This implementation should include a class that implements the same public methods as the suncertify.db.Data class, although it will need different constructors to allow it to support the network configuration.

This class is used by DataService class and the UI uses the latter. Is this a good way?
Thanks again for all the valuable advice.
Michael

[Peter den Haan]

Originally posted by Michael Tu:
1) Change the method signatures in DataInterface to throw IOException and DatabaseException.

Sounds good. Not all of them need to throw DatabaseException.

2) Not implement the LockManager as Singleton.[...]

Looks good to me.

The ClientData class is implemented merely for this requirement: This implementation should include a class that implements the same public methods as the suncertify.db.Data class[...]

That's not a reason to put it in your design -- it doesn't really do anything, and for that reason alone it should be junked. If that meant you'd have to ignore the requirement, that is what you should do, carefully documenting the reasons why. But as it happens, you're satisfying the requirement without this class! You see, it says "should include". It doesn't say that you have to be the one writing it. The stub for RemoteData generated by rmic satisfies the requirement on all counts.
- Peter

[Michael Tu]

You see, it says "should include". It doesn't say that you have to be the one writing it. The stub for RemoteData generated by rmic satisfies the requirement on all counts.

This is the first time I see this smart and valid point in this forum. The majority of people here use the way I use because of the same reason I metioned. Having such a class will do no harm though it is not necessary.
When I implement sth, I am always bugged by this question : does the assessor accept this approach? Quite often something is done not for the project but merely for the assessors. Maybe myself think so but the assessors don't. Who knows?

Really glad to have your help. Do point out if anything else not good.

Michael

[Michael Tu]

Oops.... Just repost the previous one.
Hi, Peter & other friends,
Just reexamine my implementation of FBNServer and RemoteData. My original approach is to instantiate LockManager in FBNServer and pass it to all RemoteData objects, i.e.

 ...
Data db = null;
LockManager lm = null;
//constructor
public FBNServer(String dbname) {
    db = Data(dbname);
    lm = LockManager();
}
//implementation of the interface methodpublic RemoteDataInterface getRemoteDataInstance() {
    return RemoteData(db, lm);
} 

Now I find it not very good as LockManager is solely used by RemoteData but declared outside of it. If future anything changed in RemoteData or LockManager, I may need to change FBNServer also. Will it be better if I remove LockManager from FBNServer and deal with it completely inside RemoteData, like:

 public class RemoteData extends UnicastRemoteObject implements RemoteDataInterface {
   ...
   private static LockManager lm = null;
   public RemoteData(Data db) {
       ...
       if (lm == null) lm = new LockManager();
       ... 
   }
} 

Future changes will be limited in RemoteData only.
Pls advice. Thanks.
Michael

[Peter den Haan]

Originally posted by Michael Tu:
Having such a class will do no harm though it is not necessary.

Personally, I've come to regard code as a necessary evil. Every single line of code that does not serve a very good purpose is harmful.

When I implement sth, I am always bugged by this question : does the assessor accept this approach?

Let me reassure you. Based on what I've seen in this forum, both as a humble participant and as a moderator, the assessors will let you pass almost regardless of what you do provided that you have thought it through and document your reasons why.

[...] Now I find it not very good as LockManager is solely used by RemoteData but declared outside of it. [...]

All RemoteData instances are produced by a single factory. All share a single Data and a single LockManager.
You could include both Data and LockManager as static fields inside RemoteData. But consider that a Factory is a creational pattern -- a Factory is about assembling stuff. My reasoning was that it was quite reasonable for a factory to have the shared Data and LockManager lying on the shelves ready for assembly. RemoteData doesn't need to be aware about the specific Data and LockManager implementations it gets; that is the responsibility of the factory.
In fact, in my implementation RemoteData was an inner class of the factory, so it had direct access to both data and lockManager fields. Not everyone is fond of inner classes though.
This is by no means the only acceptable solution. Static fields will do fine too. Follow your own line of reasoning there.
- Peter
PS. Can you delete your earlier duplicate post? It does weird things with the layout. Just edit it, tick the Delete Post? checkbox, then click Edit Post.
[ December 17, 2002: Message edited by: Peter den Haan ]

[Eddie Sheffield]

Originally posted by Peter den Haan:

Why bother? Just have

interface RemoteDataInterface
extends DataInterface, Remote {
   // that's all folks
}

Don't you have to explicitly rename all the methods and explicitly throw RemoteException? I tried this suggestion myself and the rmi compiler complained about the methods not throwing RemoteException.
I would really like to do it this way myself. Is there a way to make the rmi compiler happy with this kind of declaration?
Thanks,
Eddie

[Peter den Haan]

Originally posted by Eddie Sheffield:
[QBDon't you have to explicitly rename all the methods and explicitly throw RemoteException?[/QB]

They must already throw RemoteException (or its superclass, IOException) in DataInterface itself.
- Peter

[Michael Tu]

I've revised my project based on the discussions obout this topic. Now, I need to justify all the choices I made. However, I am not very sure on some issues:
1) A client may stay connected throughout or may work in cyclic mode of connection-transaction-close. Both have drawbacks. For the former, it is for sure a nightmare to the limited capacity of RMI server if nobody drop connection. The latter requires a lot of reconnections and will soon become intolerable if you succeed one but fail all other. Which one shall use???
2) For local mode, is it really not necessary to implement lock/unlock? Or is it because the locking approach for network mode not applicable here? There are many posts about this but none fully conviced me.

Pls do point out any mistake, flaw and inelegance in my design. I am planning to submit it soon if you guys feel it is ready.
Many thanks...

Michael

[Peter den Haan]

Originally posted by Michael Tu:
1) A client may stay connected throughout or may work in cyclic mode of connection-transaction-close. Both have drawbacks.

Assuming you are using RMI, the answer is simple IMHO. Stay connected. RMI has been designed to abstract remote access to long-lived objects. Leave it up to the RMI implementation to optimise this process and to manage network connections as necessary. This is exactly the kind of low-level stuff that RMI is abstracting away from you, that you shouldn't have to bother about.
Of course, abstractions only go so far and in a high-volume enterprise-level application you might well have to worry about this issue. But certainly not in the FBN application.

2) For local mode, is it really not necessary to implement lock/unlock?

That one of the design decisions you will have to make, isn't it? You can argue either way.
Certainly, you can put up a reasonable argument that local mode is inherently single-user and does not need any locking. For the FBN application this is certainly true. Applications for which this assumption does not hold would have to use the networked server; such applications are likely to be relatively complex anyway so that the extra work does not really matter.
The assessor had no problems accepting this line of reasoning from yours truly.
- Peter

[Michael Tu]

Peter, Thanks for your many helps.
I've just finished the design choice document, readme.txt, etc. I am going to submit the following in a jar file:
1) Original assignment jar file downloaded from Sun site;
2) All .java files for all packages;
3) All .class files in proper directories;
4) All javadoc files (*.html, *.css, etc);
5) designchoices.txt;
6) readme.txt.
Do point out if I miss something here.
In the assignment instruction, there is an item: user documentation (10 marks). What's this? Does it mean user manual, or more than that? Right now, I don't have a user manual nor a built-in help system. If it is necessary, shall I go for a manual or a help system? To build a help system, it requires components outside of the standard JDK package. Is this an offence?
Pls advice. Thanks.

Michael

[Peter den Haan]

I wrote a simple user manual in HTML, and used an HTMLEditorKit to display it (read-only) in an application window. The help document started with an index in HTML -- I didn't bother with a separate index pane, history buttons and the like.
It works with the J2SDK and provides adequate functionality with two dozen lines of code.
- Peter

[Michael Tu]

HTMLEditorKit to display it (read-only) in an application window

In your submission, did you include this component together with all your own files?
I am thinking about javax.help package. However, since this package is not included in JDK and must be installed separately, it definitely needs to be included in the submission file, which I think may not be acceptable to this project. What's your opinion on using javax.help?
If don't use outside tool, seems there are only two simple choices:
1. Make a standalone HTML or TXT manual.
2. Make a text manual and use sth like JEditorPane to display it - thus make it online.
Can I go for one of them?
Pls guide. Thanks.

Michael

[John Lee]

I use no. 2.

[Michael Tu]

I've settled it using the same. BTW, how did you put the server side help? My server has no GUI and is stopped by Ctrl+C. I don't know if this is acceptable. Pls comment.
Thanks & Merry Christmas
Michael

[John Lee]

Hi Michael:
I don't have gui on server site either. Since user help is for user, so it is understandable to have it only on client side. Anyway, I passed.
Merry Christmas and Happy New Year!

[Daniela Ch]

see thread :
https://coderanch.com/t/182587/java-developer-SCJD/certification/throwing-IOException-instead-RemoteException
[ December 27, 2002: Message edited by: Daniela Ch ]