Map or Hashtable

[Sri Addanki]

Hi,
while i was reading this link:
http://www.geocities.com/developergrp/guideline.htm
i found:
"...Similarly do not directly refer to a Hashtable, instead refer to a Map with Map m = new Hashtable() ; This allows you to switch from a Hashtable to a HashMap at any time, without affecting anything else."
Is this the right way? If so why?
thanks,
sri

[John Smith]

"...Similarly do not directly refer to a Hashtable, instead refer to a Map with Map m = new Hashtable() ; This allows you to switch from a Hashtable to a HashMap at any time, without affecting anything else."

Well, what is says is what is otherwise known as "program to an interface, not to implementation", -- it's easier to change your code this way. However, you don't want to use Hashtable in the first place, -- it's an older collection that has been replaced by HashMap, which you can make synchronized, if you want to.
Eugene.

[Peter den Haan]

Indeed. The idea is that you make as few assumptions about the objects you work with as possible. If you need a List, then use List, not Vector or LinkedList or ArrayList.
Why? For one, because it allows you to optimise your application later. You may be using your List in a way that is much more suitable for a LinkedList than an ArrayList. Or, you may discover that you need some special behaviour in your list and implement your own List. Good object-oriented design is, among other things, about reducing the coupling between your classes and other components as much as possible.
A word of warning about synchronization. DO NOT USE SYNCHRONIZED COLLECTIONS. Do the synchronization yourself. If, after figuring out the synchronization needs of your classes, you find that you could just as well use a synchronized collection, by all means do that. But I'm willing to bet that a synchronized collection would not do what you need it to do, and only lure you into a false sense of security: "I'm threadsafe because I use a synchronized collection" -- well, doh, no, that's not how it works at all.
- Peter

[Sri Addanki]

Thanks Peter.
I'm using the default method signature for lock:
public void lock(int record) throws IOException,
i use only the record number to lock.
So, do u still think its wise to use lockKeeper like this for storing the locked records.
private Map lockKeeper = new Hashtable();
Also in criteriaFind i use a vector to retrieve records. Is that ok? or should i use arraylist?
please comment on this,
thanks,
sri

[Peter den Haan]

Originally posted by Sri Addanki:
I'm using the default method signature [...] i use only the record number to lock.

As long as you satisfy the requirement that an unlock() call for a record held by another client is ignored.

So, do u still think its wise to use lockKeeper like this for storing the locked records.

Yes. In fact I'd abstract the whole locking business into a separate class (call it LockKeeper or LockManager). In a good OO design, each class has a single responsibility, it does exactly one thing and does it well.

Also in criteriaFind i use a vector to retrieve records. Is that ok? or should i use arraylist?

Oh, this is a religious issue, so some will take exception to what I'm going to say now. My basic recommendation is never to use Hashtable, Vector and Enumeration unless your application needs to run on JDK 1.1 or you are using some API that needs to use them.
Two reasons.
First, the new-style Collection classes are superior in a number of respects. They fit in a consistent, elegant framework. They are not synchronized which is an advantage if you don't need thread safety, and also an advantage if you do need thread safety! (see my earlier posting above). Finally, their APIs are smaller and more elegant (retrofitting Vector and Hashtable with the Collections API has made a mess of them).
Second, they are effectively if not formally deprecated. In earlier discussions I tracked down a number of quotes from the Sun documentation and tutorials which implied as much, stating that you should always use Collection classes for new development. So why aren't Vector and Hashtable formally @deprecated? Because at the time they were too deeply entrenched in existing APIs, that's why. For example, the Servlet API retained compatibility with JDK 1.1 for a long time and was forced to use Enumeration. There are numerous other examples. Sun could hardly deprecate stuff that they were still actively using elsewhere. And by now these classes have been in active use for so long that deprecating them would cause more disruption than it's worth.
- Peter

[Sri Addanki]

Hi Peter,
Thanks for that input.
I have a few more questions.
As you said:
"...In fact I'd abstract the whole locking business into a separate class (call it LockKeeper or LockManager). In a good OO design, each class has a single responsibility, it does exactly one thing and does it well...."
I have my lock/unlock method implemented in a separate class called lockmanager class .
As i said my lock method uses
only recNum as a paramter. And i don't use any clientID. Hence i made this lockmanager class extend Data class. is this OK?
So, my design goes like this:
DATA...defines...criteriaFind() implements... DATAINTERFACE
DATAINTERFACE ... has all method signatures
LOCKMANAGER...extends...DATA implements... DATAINTERFACE
REMOTEINTERFACE.. extends... DATAINTERFACE, REMOTE
RMISERVER... implements... REMOTEINTERFACE
thanks,
sri

[Peter den Haan]

Originally posted by Sri Addanki:
As i said my lock method uses only recNum as a paramter. And i don't use any clientID.

So are you satisfying the javadoc for unlock()? I bet you aren't.

Hence i made this lockmanager class extend Data class. is this OK?

Sort of. I assume its name is not LockManager but something like MultiUserData or whatever. However, because this locking stuff is really only relevant for remote implementations, your RmiServer class might be a happier place for it.
Also it is difficult to see how a lock manager taking only the record number can satisfy the requirements.
- Peter

[Sri Addanki]

Originally posted by Peter den Haan:
Sort of. I assume its name is not LockManager but something like MultiUserData or whatever. However, because this locking stuff is really only relevant for remote implementations, your RmiServer class might be a happier place for it.
- Peter

Thanks again, Peter.
Yes you are right, as far as the class name is concerned.
But still i don't understand why locking should be on the server class? I know locking is used only in remote mode, but still i feel locking is concerned with data. Hence it should be in Data subclasses. So lockManager or whatever class should extend the data class.
What do you say?
And as far as the lock method is concerned, i didn't quite understand this statement of yours:

So are you satisfying the javadoc for unlock()? I bet you aren't.

Can you explain this.
Or is there any other post related to this, so that i can peek into that.
thanks,
sri

[Peter den Haan]

Originally posted by Sri Addanki:
i feel locking is concerned with data. Hence it should be in Data subclasses.

That strikes me as a reasonable point of view. Keep things the way they are and don't forget to document this later as a design decision.

So are you satisfying the javadoc for unlock()? I bet you aren't.

What I'm referrint to is that unlock() is Ignored if the caller does not have a current lock on the requested record. Which means that unlock(), and by extension lock(), has to be aware of the locker's identity.
- Peter

[Sri Addanki]

Hi Peter,
Thanks alot again!
I implemented lock/unlock method, just exactly as said by Mark, in one of the posts.

Don't think of this locking schema as a record lock as you know in Oracle or DB2, or other DBMS. This is a self imposed "Lock", which is just that the record number is in a HashSet, and you should wait till that user removes it from the HashSet when it calls unlock()
As to your first point, lock is synchronized, when a client calls lock it either locks the record but putting the record number into the HashSet. If another client has the lock, (That record number already exists in the HashSet), then you call wait(), by calling wait, you allow another thread to call lock() which might be requesting a lock on a different record, that is not in the HashSet, therefore that second client gets the lock for a different record.

Is this fine?
my unlock goes like this:
public void unlock(int record)
{
synchronized(this)
{
if(record == -1)
dbLocked = false;
else
{
Integer i = new Integer(record);
lockKeeper.remove(i.toString());
}
notifyAll();
}
}
thanks,
sri

[Sri Addanki]

Hi Peter,
I was just reading your posts on lock, and found this:

In local mode, the database is single-user, so there is no-one else who could possibly access "your" records. Unless you want to work with multiple threads within the same JVM that are contending for row locks, there is no point in implementing any locking code.
- Peter

So, does this mean if i consider multi-threading in local mode and then i need to implement lock/unlock in local mode tooo...??
thanks,
sri

[Peter den Haan]

Originally posted by Sri Addanki:
I implemented lock/unlock method, just exactly as said by Mark, in one of the posts.

There's probably a context to this. I think this is not Mark's own implementation, or necessarily the one he considers the best one. In particular, I miss any concept of client identity in the quote, and in your code.
Your code:

public void unlock(int record)
{
    synchronized(this)
    {
        if(record == -1)
            dbLocked = false;
        else
        {
            Integer i = new Integer(record);
            lockKeeper.remove(i.toString());
        }
        notifyAll();
    }
}

Has a number of problems, I'm afraid. Two general recommendations would be to replace the synchronized block by method synchronization, and not to bother converting the Integer i into a String.
But the biggie is that this code fails to implement the javadoc documentation for it: Ignored if the caller does not have a current lock on the requested record. To do this, unlock() needs to be aware of client identity.
- Peter

[Peter den Haan]

Originally posted by Sri Addanki:
So, does this mean if i consider multi-threading in local mode and then i need to implement lock/unlock in local mode tooo...?

Yes. So by all means, do NOT consider multi-threading in local mode, and document that all threads accessing the same DataInterface implementation will share each other's locks.
- Peter

[Sri Addanki]

Hi Peter,
Yes you are right. After reading your post, i read the instructions on unlock again, here it goes:

The unlock method simply removes the lock from the specified record. If an attempt is made to unlock a record that has not been locked by this connection, then no action is be taken.

Does this mean that there should be client identity?...."If an attempt is made to unlock a record that has not been locked by this connection, then no action is be taken. "
So how should i change my code now. I see that in the other post you said that there's something called clientId, let me check that post again, before asking you any further questions.
Thanks for the input.
thanks,
sri

[Sri Addanki]

Originally posted by Peter den Haan:
Yes. So by all means, do NOT consider multi-threading in local mode, and document that all threads accessing the same DataInterface implementation will share each other's locks.
- Peter

I still feel multithreading should be considered in local mode, can you please explain why should NOT i consider this, again?
thanks,
sri

[Peter den Haan]

Originally posted by Sri Addanki:
I still feel multithreading should be considered in local mode, can you please explain why should NOT i consider this, again?

In my career so far, I have seen few applications that were not multi-user applications but still needed multiple transactions. And in all cases they were pretty large and complicated. So this requirement is rare.
You can easily get the functionality by using the networked mode server classes. You don't even have to actually use networking! -- if you use the ConnectionFactory approach, it will work just as well when local threads get Connection objects using ordinary method calls. Or, alternatively, you can simply run the networked server on the same machine. So this requirement can be satisfied anyway.
We have a requirement that is rare, can be satisfied with the existing networked server, would be quite a bit of work to satisfy otherwise. To add insult to injury, adding such code will not affect your score at best and may even decrease your score if the assessor feels you over-engineered your server. This seems like a no-brainer to me.
- Peter

[Sri Addanki]

Hey Peter,
Thanks for that explanation!
Now comes the designing part, can you please look into this:
DATA...defines...criteriaFind()
LOCKMANAGER....extends...DATA
DATAINTERFACE .... has all method signatures
REMOTEINTERFACE.. extends... DATAINTERFACE, REMOTE
RMISERVER... implements... REMOTEINTERFACE...extends...UNICASTREMOTEOBJECT
DATAACCESSLOCAL....implements... DATAINTERFACE
DATAACCESSREMOTE...extends...UNICASTREMOTEOBJECT...implements...DATAINTERFACE
DATACLIENTFACTORY.............has local,remote factory methods.
Now to create a unique client-ID i'm modifying like this.
-------------------------------------------------------
RMISERVER....has getConnection method like this:
public DataInterface getConnection() throws RemoteException
{
return DataAccessRemote data = new DataAccessRemote();
}
-------------------------------------------------------------------
REMOTEINTERFACE.....has the method declaration for getConnection();
------------------------------------------------------------------
DATACLIENTFACTORY....has getAccess() like this:
if(serverName.startsWith("rmi://localhost/"))
myServer_ = (RemoteIData)Naming.lookup(serverName);

else
myServer_ = (RemoteIData)Naming.lookup("rmi://localhost/"+serverName);
myServer_ = myServer_.getConnection();
-------------------------------------------------------------------
DATAACCESSREMOTE........has a constructor like this:
public class DataAccessRemote
extends UnicastRemoteObject implements DataInterface,
Unreferenced
{
//LockManager extends Data class
private static LockManager lockData;
public DataAccessRemote(String dbFileName) throws
RemoteException, java.io.IOException
{
lockData = new LockManager(dbFileName);
}
//other data methods
}
--------------------------------------------------------------------
DATAACCESSLOCAL........has a constructor like this:
public class DataAccessLocal implements DataInterface
{
private Data myData;

public void DataAccessLocal(String dbFileName) throws java.io.IOException
{
myData = new Data("db.db");
}
//other data methods
}
--------------------------------------------------------------------
Is this right? and also how to implement unreferenced?

Also i see that RMISERVER and DATAACCESSREMOTE have all the same methods except for getConnection(). Is that right
thanks,
sri
[ December 19, 2002: Message edited by: Sri Addanki ]

[Peter den Haan]

Originally posted by Sri Addanki:
DATAACCESSLOCAL....implements... DATAINTERFACE

What does DataAccessLocal do what Data itself doesn't do?

dATAACCESSREMOTE...extends...UNICASTREMOTEOBJECT...implements...DATAINTERFACE

This class has an odd smell about it. Shouldn't it implement RemoteDataInterface?

if(serverName.startsWith("rmi://localhost/"))

Does that mean that you're forcing client and server to work on the same host? That seems entirely wrong to me.

private static LockManager lockData;

You are initialising a static variable in the constructor as if it were an instance variable. Surely that is wrong. You're also passing the dbFileName to the constructor every time, even though you need it just once. The entire DataAccessRemote instantiation process is less than elegant; a factory class (which could have a LockManager instance variable) may well improve things.

public class DataAccessLocal
implements DataInterface {
private Data myData;

I'd make myData a final variable, although not everyone would probably agree with that. To me, it makes the code more expressive -- it tells whoever reads the code that you are initialising this variable in the constructor and are not planning on changing it, ever.
But I've got a more fundamental issue with this class, as mentioned above. It doesn't do anything!
- Peter

[Sri Addanki]

Hey Peter,
Can you please look into this again:
1.What does DataAccessLocal do what Data itself doesn't do?
I don't make DATA Class implement DATAINTERFACE.
I thought, let DATAACCESSLOCAL implement DATAINTERFACE, this way it also acts as a proxy.
What do you say
2. DATAACCESSREMOTE-This class has an odd smell about it. Shouldn't it implement RemoteDataInterface?
Yes, i think so. I'm not sure, as this is my ConnectionObject.
3.The entire DataAccessRemote instantiation process is less than elegant; a factory class (which could have a LockManager instance variable) may well improve things.
Yes, i do have a factory class called RMISERVER and an instance of LockManager is created here.
can you tell me how to instantiate the connectionobject class and pass this instance of LockManager?
4. I'd make myData a final variable.
thanks.
5.Does that mean that you're forcing client and server to work on the same host? That seems entirely wrong to me.
No, i'm not doing that.

thanks for your inputs,
sri

[Peter den Haan]

Originally posted by Sri Addanki:
I don't make DATA Class implement DATAINTERFACE.
I thought, let DATAACCESSLOCAL implement DATAINTERFACE, this way it also acts as a proxy.

What do you gain by that?

Yes, i do have a factory class called RMISERVER and an instance of LockManager is created here.

Cool, so in RmiServer you can simply create a new LockManager(data), and data can be a final instance field of LockManager. Or, alternatively, LockManager could be an inner class of RmiServer (I hear gagging sounds from some in the audience ).

No, i'm not [forcing client and server to be on the same host].

How does that square with all the stuff saying "rmi://localhost" in the code you posted earlier?
- Peter

[Sri Addanki]

Originally posted by Peter den Haan:
How does that square with all the stuff saying "rmi://localhost" in the code you posted earlier?
- Peter

Okay, Peter here it goes:
In my RMISERVER class
----------------------
I start the registry with LocateRegistry.createRegistry(1099).
and rebind this, like this:
Naming.rebind(args[0], dataServer);
---------------------------------------
Sorry, my Naming.lookUp is Now like this:
if(serverName.startsWith("rmi://"))
myServer_ = (RemoteInterface)Naming.lookup(serverName);
else
myServer_ = (RemoteInterface)Naming.lookup("rmi://"+serverName);
thanks for the correction,
sri
[ December 20, 2002: Message edited by: Sri Addanki ]