I need design advice

[xiao gao]

Hi all
Here is my design , I need all your advice!!!
I modify the Data class and put all its public methods into IData interface. I also make Data class extends UnicastRemoteObject and make IData interface extends remote. I use RMI. At the same time I have another similar class called localData and ILocalData which haven't remote functions. I make the Data singleton instance. Above is the my remote data server and local data server.
In client side, I have a DataAccessObject using adapter pattern to access the remote or local data server according to the command line paramerters. That DataAccessObject will delegate the data access from gui to the remote or local server which is not visiable to the gui. I also put lock book unlock process in that DataAccessObject.
As to the gui, I use GidBagLayout , combobox , JButton, JTable and JOptionPane. I use MVC pattern via AbstractTableModel to seperate the data from JTable, I also use Observer pattern to refresh both search and book data.
I don't track user id, but just lock the record number in a vector within Data class. Each time, client book a record, my app will first check whether this record has been in vector or that db has a -1 lock , if the answer is yes, it will wait until be notified. If this record is not in vector or no -1 lock, app will add record number into vector then read recod modify record then remove the record number form vecter(unlock), finnly call notifyall.
BTW, I have a interface called IndexOfFields, as it's name, I put all the index of fields array into that interface like
"public static final int INDEX_ORIGIN = "3";
ublic static final int INDEX_DESTINATION = "4"; Using this interface, I can locate the value of the DataInfo object easily but I wonder is this interface acceptable?
All above is my desgin and I hope to get all you advice.

[Peter den Haan]

Originally posted by xiao gao:
[...] I also make Data class extends UnicastRemoteObject [...]

Which means that it uses sockets. You cannot use UnicastRemoteObject and still prevent RMI from opening server sockets. So you can't use this object in local mode, forcing you to do this:

At the same time I have another similar class called localData and ILocalData which haven't remote functions.

Sounds like cut and paste galore to me... never a good way to get kudos in the OOP guru department.

I make the Data singleton instance.

Bad move. Remember, you're coding for reuse. How many database-driven projects will only need one single database table?

In client side, I have a DataAccessObject [...] I also put lock book unlock process in that DataAccessObject.

How do you implement proper locking semantics if the locks only live on the client side?

I don't track user id, but just lock the record number in a vector within Data class.

Do the locks live on the server after all? Anyway, two things. First, a Vector is inappropriate for three reasons. (1) It is unnecessarily synchronized. (2) It is a misleading expression of the problem. A Vector, like any List, represents an ordered collection of possibly duplicate elements. Your locks are an unordered collection of unique elements, indicating a HashSet. (3) A Vector has O(n) performance for deletion and searching; a HashSet has O(1) performance, making it much more scalable. Second, there is in (my copy of) the assignment an requirement that duplicate calls to lock() should have no ill effect. How do you satisfy that without client ID?

BTW, I have a interface called IndexOfFields, as it's name, I put all the index of fields array into that interface like "public static final int INDEX_ORIGIN = "3" [...] I wonder is this interface acceptable?

The idea is fine, but (1) most prefer a class rather than an interface (2) if you use an interface, "public static final" is implicit and Sun's coding guidelines encourage you to omit it (I don't necessarily agree with this).
- Peter

[xiao gao]

Hi, Peter
Thanks a lot , I will modify my code according your excellent advice.
Yes, I just copy and paste the Remote Data to the local Data class, it really is not good way.
I do implement the locking semantics server side, but I leave the locking control to the client. At client side , I have a Client Data Access object which implements ILocalData interface.
ILocalData has the same public methods as IData class. DAO wrap public methods of IData and ILocalData with its own public methods, so it seems to use a proxy pattern. For example, if a client call the findCriteria in DAO, DAO will delegate findCriteria to IData or ILocalData findCriteria implementation depending on current network model.
In both two interfaces(ILocalData, IData) I have three public methods associated with lock operation, they are lock, unlock and bookFlight, though there is no need to keep locking in non-network model.
If a client under network model try to book a flight, first it will get the instance of client DAO , second call DAO bookFlight method. I put DAO lock, remote bookFlight and DAO unlock methods in DAO bookFlight method. This DAO lock method calls remote lock method to lock db, then DAO will call remote Data bookflight method to make book, finally its won lock method delegated to remote unlock method to release the lock. You can see, I put lock control client side but its real implementation is server side.
As to sovle the duplicate calls to lock() without ill effect, my solution is one client lock record 10 to make booking, he add new Integer(10) into HashSet, at the same time, another thread also want to book 10 flight, it will check Hashset , if find 10 in the HashSet, that thread will call wait until first thread finish booking and call notifyAll to waken waiting thread. So you see no need for client id.
Do you think my lock semantics acceptable?
I will use HashSet intead of Vector for your comment.
Thanks peter , I am looking forward to getting your advice again.

[Peter den Haan]

Originally posted by xiao gao:
Yes, I just copy and paste the Remote Data to the local Data class, it really is not good way.

Consider implementing your networked RemoteData as either a subclass of, or a wrapper around, Data. Create an interface that both implement. The advantages are as follows. (1) One code base - no cut and paste. (2) A natural place to put the locking logic and everything else to do with multi-user handling: RemoteData. (3) The client can access both Data and RemoteData through the interface, probably without using any (I*Data) adapter objects.

DAO wrap public methods of IData and ILocalData with its own public methods, so it seems to use a proxy pattern. For example, if a client call the findCriteria in DAO, DAO will delegate findCriteria to IData or ILocalData findCriteria implementation depending on current network model.

I can't remember the precise wording, but the instructions state that you are supposed to create an object that implements the same methods as Data. This doesn't make a lot of sense unless you interpret it as a requirement that the object implements the same interface as Data. Unless I'm misunderstanding your design, this element is missing.

though there is no need to keep locking in non-network model.

Here I agree completely. As a consequence, the presence of lock() and unlock() in Data is nonsense if you are using an adapter anyway. Either scrap your adapter, or scrap the locking methods from Data. Can you justify the adapters?

You can see, I put lock control client side but its real implementation is server side.

So the local implementation merely delegates (which is what you'd expect an adapter to do). The good thing about using adapters in networked mode is that you can satisfy the lock() and unlock() requirements without modifying the method signatures - either you delegate to server-side methods that take a client ID, or you filter out "erroneous" lock()/unlock() calls in the adapter. Either method is viable.

[...] another thread also want to book 10 flight, it will check Hashset , if find 10 in the HashSet, that thread will call wait until first thread finish booking and call notifyAll to waken waiting thread. So you see no need for client id.

Three things. First, what happens if client #1 would call lock(10) again, even though it still has that lock? Similarly, what happens if client #2 would try calling unlock(10) even though it doesn't own the lock? Finally, be sure to take care over synchronizing this procedure. Keep your HashSet though - in this forum and elsewhere I have been recommending repeatedly to use unsynchronized collections because many developers are lured into thread-unsafe programming by the "safety" of Vector and Hashtable.
- Peter

[xiao gao]

Hi all
I have re-designed my project after studying the great advice from Peter den Haan.
Here is my design,
Server side, I just modified the Data class and put all its public methods in IData interface and make these method throw remoteException. I have another interface called IRemoteData which extends IData and Remote. I use a class called RemoteData implementing IRemoteData as remote server which wraps public methods in Data with its own public methods since two class share the same interface. I bind the RemoteData to the registry and make data transparant to the client. Simply, I still don't plan to track userId instead I use a Collections.synchronizedSet(new HashSet()) to return a thread-safe set to keep record number, if one client try to book flight 1, record number of flight 1 will be put into that set, if no same number in that set before, or this thread will wait until be notified.
Client side,
I use a class called clientData which implements Idata interface. This class will use adapter pattern to get remote data reference or local data reference via different constructors. As to the local Data ,I user Data class directly by calling its public methods. clientData object will wrap remote or local data public methods with its own public methods for the same interface all of them.
GUI,
As to the gui, I don't change a lot, still use GidBagLayout , combobox , JButton, JTable and JOptionPane. I use MVC pattern via AbstractTableModel to seperate the data from JTable, I also use Observer pattern to refresh both search and book data.
Above is my design , I am always looking forward to getting any great advice from all of you, Thank you again.

[mike seluker]

My design is almost similar to u... the only thing is that i'm using hashmap ( to store client id)..
How u have taken care when record = -1. I mean whole DB needs to be locked?

Originally posted by xiao gao:
Hi all
I have re-designed my project after studying the great advice from Peter den Haan.
Here is my design,
Server side, I just modified the Data class and put all its public methods in IData interface and make these method throw remoteException. I have another interface called IRemoteData which extends IData and Remote. I use a class called RemoteData implementing IRemoteData as remote server which wraps public methods in Data with its own public methods since two class share the same interface. I bind the RemoteData to the registry and make data transparant to the client. Simply, I still don't plan to track userId instead I use a Collections.synchronizedSet(new HashSet()) to return a thread-safe set to keep record number, if one client try to book flight 1, record number of flight 1 will be put into that set, if no same number in that set before, or this thread will wait until be notified.
Client side,
I use a class called clientData which implements Idata interface. This class will use adapter pattern to get remote data reference or local data reference via different constructors. As to the local Data ,I user Data class directly by calling its public methods. clientData object will wrap remote or local data public methods with its own public methods for the same interface all of them.
GUI,
As to the gui, I don't change a lot, still use GidBagLayout , combobox , JButton, JTable and JOptionPane. I use MVC pattern via AbstractTableModel to seperate the data from JTable, I also use Observer pattern to refresh both search and book data.
Above is my design , I am always looking forward to getting any great advice from all of you, Thank you again.

[xiao gao]

Yes, I wrap this -1 in the lock method