Final review - PLEASE READ

[Chiji Nwankwo]

Hi all,
Please go over my review and let me know any comments or critisms you might have.
NOTE: I HAVE OMITTED THE METHOD'S PARAMETERS FOR BREVITY.
suncertify.client
-----------------
public class BookingDetail
Class used to encapsulate all the details of a booking, such as flight number, seats, booking date and status (Confirmed, Unconfirmed ...).
public class ClientLauncher
Sets the look and feel of the application and starts off the client application.
public class FlightDetail
Class used to encapsulate all the details of a flight, such as flight number, origin airport, destination airport, etc..
public class TravelAgency
Facade that implements all the business methods, such as searchFlight() and bookFlight().

suncertify.client.ui.controller
-------------------------------
Most of the buttons and menu items contained within the GUI application are represented by seperate actions, whose names give an indication of the role they play. For example the 'book' button's interaction is handled by a 'BookAction' instance. All the Action instances are declared external to the GUI. Most of the action instances are passed a reference to a controller (see controller definition below)object.
public class MainController
This class serves as the controller (in the context of MVC) for the GUI application. Its primary purpose is to create a connection between the view and data parts of the GUI application. The presence of this class abstracts the view from the data parts of the application. This class contains methods which can be used to set data contained within the model.

suncertify.client.ui.model
--------------------------
public interface ClientModel
Defines all the operations that can be used to access the state of the main GUI application. This interface is 'accessed' by the view of the GUI application. Only accessors are provided in this interface. This is the interface that is used by the GUI application, which means that the GUI application (view) cannot modify the state of the model directly.
public class ClientModelEvent extends EventObject
ClientModelEvent is used to notify listeners that a client model's state has changed.
public interface ClientModelListener extends EventListener
Defines the interface which is implemented by objects that are interested in receiving notification changes that occur in the ClientModel object.
public class DefaultClientModel implements ClientModel
This class serves as a default implementation of the ClientModel interface, which is used by the main GUI application. This class also provides mutators that can be used modify the state of the model. This class is only accessed from with the MainController class (defined above).
public class ResultsTableColumnModel extends DefaultTableColumnModel
A custom DefaultTableColumnModel implementation used by the main GUI application.
public class ResultsTableModel extends AbstractTableModel
A custom AbstractTableModel implementation used by the results table which is displayed as part of the main GUI application.

suncertify.client.ui.view
-------------------------
public class BookingDialog extends JDialog
A dialog box that is used to display all the details of a flight that has been selected by the user. This dialog box gives the user the option to
cancel or proceed with a booking.
public class ConnectionDialog extends JDialog
This the first dialog box presented to the user when the client side of the application starts up. The dialog has options that allow the user to specify the name of a database, when in local mode and the host name/ip and rmi port,
when in remote mode.
public class GUIMediator
A mediator, that coordinates the interaction between most of the components contained within the main GUI application.
public class MainView extends JFrame implements ClientModelListener
Main GUI, which is notified when a change in the model's state occurs.
public class UserGuideDialog extends JDialog
Dialog in which the user guide is displayed. This dialog is invoked by the user guide menu item in the MainView GUI.

suncertify.db
-------------
public class Data implements DataInterface
Modified data class to provide criteriaFind() method and fixed deprecated methods. CriteriaFind method is capable of searching for any number of
fields provided to it.
public class DatabaseException extends Exception
Unmodified except for javadoc comments.
public class DataInfo implements Serializable
Unmodified except for javadoc comments.
public interface DataInterface
Exposes methods that can be used by both local and remote applications. The methods in this interface throw both DatabaseException and IOException.
public final class DataInterfaceFactory
This class creates and returns remote or local DataInterface implementations. Exceptions are rethrown by this class as a custom application exception, which is caught in the in the MainController class. The MainController class
is responsible for displaying the correct dialog box with the relevant error message.
public class FieldInfo implements Serializable
Unmodified except for javadoc comments.

suncertify.server
-----------------
public interface DataService extends Remote, DataInterface
Interface that is exported to clients of the remote system.
public interface DataServiceFactory extends Remote
Remote object registered with the RMI registry. Contains a single method, getDataService(), which is responsible for creating and returning DataService instances to the client. Each client will get a unique DataService implementation.
class DataServiceFactoryImpl extends UnicastRemoteObject implements DataServiceFactory
A DataServiceFactory implementation responsible for creating and returning DataService objects. This class is also responsible for creating Data and LockManager instances and allocating them to the new created DataService implementation. Several DataService implementations can share the same Data and LockManager instances. Each newly created Data instance has a corresponding LockManager instance tied to it within the DataService implementation.
class DataServiceImpl extends UnicastRemoteObject implements DataService, Unreferenced
A DataService implementation, exported to clients of the remote system which is used to wrap calls made to the database file. Instances of this class, which are created by the DataServiceFactoryImpl, contain a reference to the
LockManager and Data instances passed in from the DataServiceFactoryImpl class. All the database file calls are delegated to the Data instance and
the lock and unlock calls are delegated to the LockManager instance.
class LockManager implements Runnable
Deals with concurrent database file access issues. Provides implementations for the lock and unlock methods. Contains an internal, daemon thread which deals with deadlocks.
public class Server
Responsible for intialializing the server's environment, binding the remote DataServiceFactory, object to the registry and starting the server. This class contains a key listener, which listens out for a 'q' or 'Q' keystroke. Either of these keystrokes can be used to stop the server application at any time. Stopping the server will unbind any remote objects bound to the registry and gracefully exit the application. The key listener runs in its own, low priority, thread.
public class ServerLauncher
Starts the server side of the application. Command-line parameters are passed to the server side of the application.
public interface Stoppable
Contains a single call back method, stop() and is implemented by classes that will need to be stopped before the application exits. I have not included which classes implement the stoppable interface in this review.

Thank you very much for taking the time out to go over my review.
Regards,
Chiji

[HS Thomas]

Chiji,
I think you may have gone towards a very detailed design and may have sacrificed simplicity in doing so. (Here, I speak for myself only).

You may need to justify a few design decisions and ask a few questions of the design.
To start off here are a few questions, not criticisms.
Why have you catered for deadlocks where a good multi-threaded solution should prevent such a thing from ever occurring?
The daemon thread should prove unnecessary.

Contains an internal, daemon thread which deals with deadlocks.

regards
[ March 12, 2003: Message edited by: HS Thomas ]

[John Smith]

Why have you catered for deadlocks where a good multi-threaded solution should prevent such a thing from ever occurring?
The daemon thread should prove unnecessary.

I certainly agree with that. If your design has a potential for a deadlock, you need to rethink your design, and not to add a patch.
Eugene.

[Chiji Nwankwo]

Hi,
I think I have sparked some confusion in my wording. My use of the word dead locks here is wrong. What I meant to say is the thread contained within the LockManager class releases locks that have been acquired by clients, who for one reason or the other died / disconnected before the lock->read->modify->unlock sequence could be completed. I hope this makes more sense.
Regards,
Chiji

[HS Thomas]

Ah , a dead lock rather than deadlock (LOL).
That makes more sense.
But if you are using Unreferenced, do you need this daemon thread?
regards

[Chiji Nwankwo]

Ah , a dead lock rather than deadlock (LOL).
That makes more sense.

Sorry about the confusion .

But if you are using Unreferenced, do you need this daemon thread?

As far as I understand, correct me if I am wrong, Unreferenced is called after the lease time on the remote reference expires. This value may vary from machine to machine and since I am not explicitly providing a value for the lease period, I didn't want a scenario where a client tries to access a record, that has been locked by a dead client and will have to wait for the lease period to expire before it can access the record. At the moment the thread within the LockManager will periodically check for locks that have exceeded a specified amount of time and release those locks, so that other waiting clients can access the record. I must also mention that the thread only checks for DEAD client's locks, when there are locks present.
Regards,
Chiji

[HS Thomas]

You are a few steps ahead of me.
But it begs the question: Are these two separate design options ?

That is 1: use Unreferenced and let the "system" decide when to release the lock for the dead client.
2: Don't specify a lease period and the "application" decides when to release the lock.
If this was the case, personally I'd rather let the "system" handle this , and free the application to do flight search and bookings.
I'm thinking along the lines of the way garbage collection can be handled.
Leave the JVM to do it by itself, or force the collecion by setting references to null.
regards
[ March 13, 2003: Message edited by: HS Thomas ]

[HS Thomas]

Or am I totally off the beaten track ?
Anyone ?

[Chiji Nwankwo]

Hi,

That is 1: use Unreferenced and let the "system" decide when to release the lock for the dead client.
2: Don't specify a lease period and the "application" decides when to release the lock.

I don't quite understand the 2 options that you have specified. Please elaborate. Is that the only part of the system that you have a query with so far. Please your comments are very welcome and thanks for the feed back.
Regards,
Chiji

[HS Thomas]

Okay, I'll come back to what I think are two design options after doing some reading up, or if someone jumps in and kindly volunteers to agree/disagree.
The other issue I have is the following.

Several DataService implementations can share the same Data and LockManager instances. Each newly created Data instance has a corresponding LockManager instance tied to it within the DataService implementation.

It could be the wording again.
All DataService implementations (client connections) WILL share the same Data but each client connection has it's own LockManager instance but shares the HashSet or whatever it is you store your unique client identifier in.
Just semantics, but the two could be implemented very differently.
regards

[HS Thomas]

unreferenced
This thread shows you are OK with having both a daemon thread and using unreferenced to handle dead client locks as long as you have given a good reason (i.e. understand the mechanics behind it).
I hope they'll accept my reasoning that this is a system requirement and not an application requirement. By system I mean the RMI distributed garbage collection sub-system.
But in the real world I will hire you rather than me to handle dead client locks.

regards
[ March 14, 2003: Message edited by: HS Thomas ]

[Chiji Nwankwo]

Okay, I'll come back to what I think are two design options after doing some reading up, or if someone jumps in and kindly volunteers to agree/disagree.

Ok, I will await your response on the subject.

It could be the wording again.
All DataService implementations (client connections) WILL share the same Data but each client connection has it's own LockManager instance but shares the HashSet or whatever it is you store your unique client identifier in.
Just semantics, but the two could be implemented very differently.

The wording is probably wrong again. I will try to explain with words and a code snippet. I am getting my syntax all mixed up it seems.
The DataServiceFactoryImpl, which is exported to the rmiregistry as a DataServiceFactory interface, class is responsible for creating DataServiceImpl, Data and LockManager instances. Each time a new client connects to the system, in remote mode, the getDataService() method of the DataServiceFactory interface creates a new DataServiceImpl instance and passes it a previously created Data and LockManager instance. The getDataService() method returns a DataService, remote, interface to the client of the system. When the server is started up, if 3 database file names are specified on the command-line then 3 LockManagers will be created to correspond with the 3 Data instances. Please see a snippet of the getDataService() method below;

  
public synchronized DataService getDataService() throws RemoteException {
// increment the value of dataCount until the max 
// number is reached then start the count from zero
if (dataCount == (maxDataCount - 1)) {
dataCount = 0;
} else {
dataCount++;
}
Data data = (Data) dataInstances.get(dataCount);
LockManager lockManager = (LockManager) lockManagerInstances.get(dataCount);
DataService dataService = new DataServiceImpl(lockManager, data);
dataServiceInstances.add(dataService);
return dataService;
}

Regards,
Chiji

[Chiji Nwankwo]

unreferenced
This thread shows you are OK with having both a daemon thread and using unreferenced to handle dead client locks as long as you have given a good reason (i.e. understand the mechanics behind it).

Thanks for the link.

I hope they'll accept my reasoning that this is a system requirement and not an application requirement. By system I mean the RMI distributed garbage collection sub-system.

I am sure your reasoning will be fine. From what I have seen so far on the forum, there have been so many mixed views and all the different views seems to have passed. I think the most important thing as you pointed out is being able to defend the solution that have implemented and the line of reasoning behind it.
Regards,
Chiji

[HS Thomas]

Hi Chiji,
Thanks for the feel-good support on leaning towards handling of dead client locks being a system requirement. There is now a third option to be considered , do nothing about dead client locks .

When the server is started up, if 3 database file names are specified on the command-line then 3 LockManagers will be created to correspond with the 3 Data instances.

This is a new slant on the use of the LockManager class.
If you search for threads on connection on this forum , you'll find lots of discussions supporting the following :
Each client creates it's own Connection object, with a connect(table name) method and it's own LockManager object with a reference to a shared HashMap/WeakHashMap for storing the unique client identifiers.

I suspect it's semantics again, and what you call LockManager , I call HashMap/WeakHashMap i.e the Collection class for storing the identifiers.
Not quite

what you call tOmato , I call tomAtoe

but more apples and oranges.
i.e. two very different things.
So MY LockManager is created on client connection , the reference to the data/tablename instance is a static final variable ensuring the same data is re-used and so does not have to be created explicitly on server start-up. The HashMap/WeakHashMap is a final reference , again it does not have to be created on server start up.
This seems to be a standard solution. Do check up on the other threads. There are several reasons why it is a good solution.
Assigning responsibilities in the right place?
Starting the server, should just start the server registry. The Client Connection class is responsible for connecting to the data , the LockManager is responsible for acquiring and releasing locks on the data and uses a HashMap/WeakHashMap to achieve this.
Looking at your code snippet , I don't think you should have data.getInstances and LockManager.getInstances in your code.
Hope that is of help .
regards
[ March 14, 2003: Message edited by: HS Thomas ]

[Chiji Nwankwo]

Hi,

Thanks for the feel-good support on leaning towards handling of dead client locks being a system requirement. There is now a third option to be considered , do nothing about dead client locks .

NO worries.

This is a new slant on the use of the LockManager class.
If you search for threads on connection on this forum , you'll find lots of discussions supporting the following :
Each client creates it's own Connection object, with a connect(table name) method and it's own LockManager object with a reference to a shared HashMap/WeakHashMap for storing the unique client identifiers.
I suspect it's semantics again, and what you call LockManager , I call HashMap/WeakHashMap i.e the Collection class for storing the identifiers.
Not quite

When you say each client what are you referring to? The factory responsible for creating DataServiceImpl instances creates the Data and LockManager instances and passes references of these objects to the newly created DataServiceImpl class. The reason I create the LockManager and Data instances in to keep track of the number of created Data and LockManager instances. The Data and LockManager instances are created when the DataServiceFactoryImpl is created, before it is registered with the rmiregistry. This is the sequence of events from the client-side of the application.

Client connects to remote server by specifying a URL (ip/host name and / or port number)

Client performs a lookup to obtain a remote reference to the DataServiceFactory

Client calls DataServiceFactory's getDataService() method to retrieve a DataService implementation

When the getDataService() method is called a new DataServiceImpl instance is created and the previously created Data and LockManager instances are passed into the constructor of the DataServiceImpl class. The Data and LockManager instances are created when the server is started up, that way if the administrator, who starts the server, provides an invalid database name and location an exception will be thrown and a message displayed on the console to this effect. By creating the Data instances in advance I am cutting down the amount of things that can go wrong when the client requests a remote DataService instance. I also create the Data instances when the server is starting up due to the fact that I don't think the user should be concerned with the fact that database name and location specified on the server side, when the server was started, was/is incorrect. Each LockManager created, contains its own HashMap of locks that corresponds to the locks stored for clients accessing the same Data instance. So in effect the DataServiceImpl class indirectly ties a Data instance to a LockManager instance. Since the server is capable of receiving 1 or more database names and locations, I create an amount of LockManager instances that correspond to the number created Data instances.

Assigning responsibilities in the right place?
Starting the server, should just start the server registry. The Client Connection class is responsible for connecting to the data , the LockManager is responsible for acquiring and releasing locks on the data and uses a HashMap/WeakHashMap to achieve this.

My Server class, is responsible for creating the registry, creating the DataServiceFactory instance, binding the instance to the registry and stopping the server. The main responsibility of DataServiceImpl class, which I think is the same as you Client Connection class, is to delegate database calls to the Data instance and lock/unlock calls to the LockManager class, among other things. I think the main difference between your implementation and mine is where and when the Data and LockManager instances are created. But as I explained, I don't see why the client of the application should be concerned with the fact the back-end system was unable to connect to the database when the client has not specified the name of a database to connect to. As far as I understand it, the user specifies the name and location of the datafile in 'local' mode and specifies only the hostname/ip and port number in 'remote' mode.

Looking at your code snippet , I don't think you should have data.getInstances and LockManager.getInstances in your code.

All the piece of code, from the previous message, is doing is retrieving a Data instance and a LockManager instance, which is passed to the newly created DataServiceImpl instance. dataServerInstances and lockManagerInstances are two arraylists used to store the previously created Data and LockManager Instances, see below

Data data = (Data) dataInstances.get(dataCount);
LockManager lockManager = (LockManager) lockManagerInstances.get(dataCount);
DataService dataService = new DataServiceImpl(lockManager, data);

The arraylists shown in the code snippet above, are created and populated when the server is started up.
Thanks for your feedback.
regards
Chiji

[HS Thomas]

Hi Chiji,
I was just pointing out that your solution is different from the standard solution given in this forumregarding LockManager use.
I am confused why you are creating Data and LockManager instances in advance. Is it for Connection pooling , which I think I am right in saying is outside the requirements ?
I think we'd have to wait for someone else to aid as what you have written sounds plausible, but not standard/fit the requirements.
This is an assignment after all, not a real world application. The more detail you build in the more you have to test, the more time your assessor has to spend checking and testing, the crosser he/she'll get and the more likely you are to loose points. Best be cautious and try and convince a few detached parties of the validity of your design first.
I must admit that I cannot judge whether it is valid in the context of the assignment.
Good Luck !

regards
[ March 14, 2003: Message edited by: HS Thomas ]

[HS Thomas]

I think the main difference between your implementation and mine is where and when the Data and LockManager instances are created

That's right.

I don't see why the client of the application should be concerned with the fact the back-end system was unable to connect to the database when the client has not specified the name of a database to connect to.

That's wrong. How else will the user know why his application failed?
How are you handling the error message, i.e. how does creating data and lock manager instances in advance help in this case ?
regards

[Chiji Nwankwo]

I am confused why you are creating Data and LockManager instances in advance. Is it for Connection pooling , which I think I am right in saying is outside the requirements ?

Did you implement multiple databases in your solution? How did you go about it?
Regards,
Chiji

[Chiji Nwankwo]

Hi,

That's wrong. How else will the user know why his application failed?
How are you handling the error message, i.e. how does creating data and lock manager instances in advance help in this case ?

I view the starting of the server and client as two seperate operations. What I have tried to do is create a distinction between the error messages that are displayed on the server side when the server is started and the error messages that are displayed on the client side when the client is started. I feel the server should be started up and made available for the clients. I think the user should be shown error messages that relate directly to the operation they are trying to achieve. When the server is started up any errors that pertain to the creating and registring of the the factory to the registry and creating the Data instances are displayed on the console. That way the server side of the application is fully available and awaiting client connections. All the factory does when the client calls the getDataService method is create and serve out remote DataService instances. I guess I could have delayed the creation of the LockManager instances until the later on, but I still think the creation of the Data instances is not entirely wrong.
Let me know what you think.
Regards,
Chiji

[HS Thomas]

Chiji,
I am with you now.

I guess I could have delayed the creation of the LockManager instances until the later on, but I still think the creation of the Data instances is not entirely wrong.

Ok, not if you see your Data instance as a database being available 24-7.
But if you are connecting to a table/view , you may want to think about lazy initialising your Data instances , also.
Similarly, if you are handling multiple databases , the database schemas, will also be encapsulated in the Connection object as parameters. A standard solution repeated in many database packages. If one understands it's power and simplicity, why deviate from it.
If you are going to use it , use it well.
There are other options that the assignment grades equally - some may even supercede the Connection /LockManager solution given time.
So keep on plugging away.
You may find these threads useful. I know I need a refresher so I am going back to searching on Connection.
regards
Topic: Can the ClientConnection be used for transaction,exception handling
connectionFactory question
[ March 14, 2003: Message edited by: HS Thomas ]

[Chiji Nwankwo]

Hi,

Ok, not if you see your Data instance as a database being available 24-7.
But if you are connecting to a table/view , you may want to think about lazy initialising your Data instances , also.

I see what you mean. Thanks for the clarification and perseverance .

Similarly, if you are handling multiple databases , the database schemas, will also be encapsulated in the Connection object as parameters. A standard solution repeated in many database packages.....

I didn't quite understand the statements you made above. Please clarify. I will use lazy initialiation as you have suggested. Supposing 2 datafile names were specified on the command-line, when starting the server, will it be wrong to give a seperate datafile name to different DataServiceImpl (your Connection object) instances when they are being created?
Thanks for your input.
Regards,
Chiji

[HS Thomas]

I see what you mean. Thanks for the clarification and perseverance .

No problems. Just doing unto others(I hope) as was done to me on this forum.
As to the other part , I was just giving pointers to info on this forum that may help.
I am repeating myself , but search this forum for the word connection .
Peter de Haan is the leading proponent for Connection with LockManager. you could search on his member id number also.
I intend doing the same for a refresher . If you want to have another bash at a later date when I am more informed, you are more than welcome.
regards
[ March 15, 2003: Message edited by: HS Thomas ]

[HS Thomas]

Chiji ,
The following thread snippet is the most relevant to the latter part of your last post.
See if it helps. Translate it to code.

regards

posted January 04, 2003 05:45 AM
--------------------------------------------------------------------------------
quote:
--------------------------------------------------------------------------------
Originally posted by HS Thomas:
I'm still trying to absorb this . So the first option would allow more than one database schema in a single JVM or many JVMs?
--------------------------------------------------------------------------------
Let me rephrase. There are broadly two approaches you can take with your ConnectionFactory (or whatever you call it)
Each ConnectionFactory corresponds to exactly one Data instance, hence one table. You gain access to a table by looking it up in the RMI registry and getting a connection to it.
The ConnectionFactory.connect() method takes a table name. Each ConnectionFactory encapsulates an arbitrary number of Data instances; the factory can be said to correspond to a database schema. You would gain access to a table by looking up the schema in the RMI registry, and requesting a connection to one of the tables in the schema. The schemas in the RMI registry can live in one or more JVMs.
Both alternatives satisfies the assignment requirements; you should do whatever is easiest and/or comes most naturally to your design.
In either case, you shouldn't go out of your way to limit the number of ConnectionFactory instances by enforcing a singleton pattern unless it significantly simplifies your design (which I don't think it will).
- Peter
--------------------

[Chiji Nwankwo]

Thanks for the tip. Could you please tell me if my line of thinking, from the quote below, is correct.

As far as I understand it, the user specifies the name and location of the datafile in 'local' mode and specifies only the hostname/ip and port number in 'remote' mode.

Am I correct in thinking that the name and location of the database file is not supposed to be supplied by the user in 'remote' mode.
I guess my major concern is with the fact that it was specified in your last post that the ConnectionFactory's connect() method, specifies a database file name to connect to.
Can you please clarify this.
Regards,
Chiji

[HS Thomas]

The ConnectionFactory.connect() method takes a table name.

This applies only in networked mode.
I think , the location of the data (the directory ?) is best specified on creating the Data instance for both remote and local.
On starting the Server, sounds like a good place to do it.
Pass the parameter in and store as a final static variable.
regards

regards
[ March 18, 2003: Message edited by: HS Thomas ]