write method
of FileObject should increment the version number. If you
call write as part of readObject, the version
number of the object that the remote client receives will be one greater
than it should be. The solution is either to decrement in readObject
or (maybe better) have both write and readObject
call the same helper that does the actually writing but doesn't do the
version increment. Some people set the version number to 0
in readObject. Why is this incorrect from the
viewpoint a directory repository?
finalize for
FileObjectImpl: It is a good idea to delete any temporary
or unneed disk files as soon as possible, but you still needed to have
a finalize method that explicitly removes the
disk file when the FileObject is garbage collected. If
you have to remove multiple files, be sure that your finalize
method catches necessary exceptions so that it doesn't abort before
finishing its cleanup. There is no guarantee that the
finalize method will ever be called. You should also
call deleteOnExit for each temporary file that you create.
Note that deleteOnExit
only deletes the file when the entire program exits.
This method guarantees that when the server exits,
all of the disk files associated with temporary files will be deleted.
You can encourage the Java Virtual Machine to run the finalizers of
objects that are pending deletion by calling System.runFinalization.
FileObject:
You should make no assumptions about the size. You can always find out
before allocated the array.
Vector of Byte objects
instead of an array of byte values: The first
implementation makes a very inefficient FileObject.
rename either
failed to preserve file contents under some circumstances or left
an extra file afterwards.
Remember rename can throw an exception. Using
rename is completely unnecessary --- just keep
the logical and physical name separate. Be sure to
delete the old file after the new one is in place. Hardcoding a
file name for the temporary file works as long as the FileObject
is synchronized.
FileObject. A file
with no data has a FileObject, but the disk file
with the data either hasn't been created or is empty.
read of a file returns as many bytes
as you have requested: Either use readFully or put your
read in a loop. (This becomes a particular issue
in the serialization functions, which appear to have a 1024 block size.)
read method of the FileObject by simply returning a pointer to the internal data structure.
Similarly others implemented the write method by setting
the internal array to the array passed as a parameter rather than copying.
This is an object-oriented programming disaster.
Hashtable but not
from the Vector. Others implemented update by
creating a new FileObject and replacing the old one. This has
several problems. The version numbers are off and because
FileObject is a remote reference. The clients who had references
before the replacement will be left in an inconsistent state.
sequence of a fixed length, if the
sequence doesn't have a length limit.
close or improperly placed close:
Several people for got to close
their input or output streams in the FileObject read
or write method. Also, some people put the close
in the same try-catch with the read or write.
try {
read...
try {
close..
}
} catch
has the same problem as:
try {
read ...
close ..
} ...
In both case, the close will not be called if read
throws an exception.
getListing is likely to be
inefficient. The Arrays class has an efficient
static sort method.
Vector class is implemented as a linked list.
The TreeSet is a nice class to look at.
Several people did an extra copy on the getListing method.
Also, for copying, consider using the arraycopy method
in the System class.
throw are informative.
For example, on getFile you should not use
File not found as the error message.
Instead, you should say something like: File Lecture22.pdf not found.
Remote exceptions in particular should be very informative, but you should
be careful not to put something in the error message that the client
should not know. Often you have more specific information about
the error than is contained in the system's exception message when
an exception occurs. It is often useful to catch the exception
and rethrow it with a more detailed error message.
Exception is thrown, the method should throw an
exception on an error. Some people bypassed the exception mechanism, for
example, by catching all exceptions on read in FileObject
and then returning null.
DirectoryImpl
can generate exceptions from many sources. In general, these exceptions
should be caught and appropriate action should be taken. If the exception
is something that the client needs to know about, then the method
should throw the exception associated with the Corba interface.
readObject:
Some people didn't save the data on disk so the object wasn't
properly reconstituted.
readObject: Some people actually had code in
readObject that prompted the user to enter the
data for the file. The data was supposed to come from the
input stream.
readObject or
writeObject: for example, making it public. Your
test programs should create an
ObjectInputStream and ObjectOutputStream
objects and call readObject and writeObject
to test the serialization.
DirectoryImpl
instead of creating a separate synchronization class is a very bad
strategy that is nearly impossible to test adequately. You should
create a separate synchronization class, say RWSync
to enforce reader/writer
synchronization. This class might have methods like startRead,
endRead, startWrite and endWrite.
(These aren't the only possible set of methods, but we'll assume these
for the discussion.) The RWSync class can be tested
in isolation so that you can be fairly confident that it works, before
using it in the remote system. In fact, if you wanted to do a really nice
job, you could define RWSync as an interface. You could
then implement particular reader/writer policy such as strong reader
in a class that implements RWSync. You can then change
policies without changing the remote repository at all. Furthermore,
you can use RWSync to synchronize both FileObject
and Directory objects.
mylock.startWrite();
do some stuff that might throw an exception
mylock.endWrite();
the write lock will not be released if the exception occurs. There
are two approaches to handling this problem:
catch and then rethrow the
exception.finally clause.finally
clause is always executed regardless of whether or not an exception
occurs and whether or not it is caught. (The Core Java Book Vol. I,
Chapter 11 has a very nice discussion about exception handling, which would
be good to reread now.)
FileObjectImpl using a synchronization object such as
RWSync, you should be careful not to serialize
it, because that will usually cause a deadlock when you unserialize.
You can prevent a field from being serialized by including the keyword
transient in its declaration.
Vector object myVector:
Vector temp = null;
mylock.startRead();
temp = myVector;
mylock.endRead();
return temp;
The variable temp is a reference to myVector,
not an actual copy of myVector. This presents particular
difficulties when temp is being returned in a remote
call, as the return statement must serialize temp.
It could be in the middle of serialization when another thread adds
something to the vector. The correct approach is to clone
the myVector object inside of the synchronization construct.
printWriter and similar classes are
not synchronized by default. You can overcome this problem by writing
your own printWriterSynchronized class that extends
printWriter and simply calls the super class method of
the same name from a synchronized method.
transient
so that it will not be serialized. Otherwise unserialization can cause
a deadlock.
update method of DirectoryObject
is a reader: Some people were confused because the update
method called the write method of FileObject.
However, as far as DirectoryObject synchronization goes,
the update is a reader. In fact, the call to
write should be outside of the synchronization completely.
FileObjectImpl: The
FileObjectImpl should really be synchronized using reader/writer
synchronization, although it wasn't required in this assignment. The file
name never changes, once the object is created, so it doesn't have to
be synchronized. However the version number and the size can change,
so getSize and getVersion must both be
treated as readers. The writeObject requires reader synchronization
while the readObject requires writer synchronization.
write in FileObject is
synchronized as a writer, you should be careful not to call write
from another object that is also synchronized (such as readObject).
stopWrite in both the catch and
the finally clause. Similarly with the stopRead.
synchronized and R/W monitor in the
DirectoryImpl: If every method in
DirectoryImpl is synchronized, the R/W will have
no affect except to deadlock the system. Be sure you understand
why.
Hashtable (which is synchronized), to test whether
the FileObject is in the table before using reader-writer
locks to actually insert the new FileObject. This code
has a race condition because another caller could add the same
FileObject to the table between the time when you tested
for the object and when you acquired the writer lock.
Why are the newer collections classes,
such as HashMap, not synchronized?
synchronized
keyword: Someone used reader/write locks for the read
and write methods of FileObject, but the
synchronizedkeyword for the getVersion.
update while writing
a FileObject: Why doesn't this make sense?
getListing:
The vector needs to be cloned. Why?
main
method for each object such as FileObjectImpl with the test
code for testing the class in isolation. You can document your tests
in the output of this main. If you make any changes to
FileObjectImpl later, you can just rerun FileObjectImpl
using its main.
remove method of Directory
was called. If you didn't mention it, I might wonder if you actually
looked.
DirectoryImpl with the name of a log file. If that
constructor was used, then the DirectoryImpl logged
its transactions to the indicated file. The logging used
PrintWriter
which is a class that catches all of its exceptions, particularly
useful for logging purposes. This logging produced a very nice record
of test results because it was actually built in to the class.
idl
with oldImplBase.