• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Data: Mutition/Singletone

 
Vlad Rabkin
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi guys,
I did have Data class mutition, not a singletone. (My all remote connection factory creates a Data instance once and put the instance in a C-tor of RemoteObject every time).
Since I do synhronize all methods on "this", I am afraid that Sun's automatic test can attemp to create may istances of Data. My locking and synchronization will not work of course.
Therefore I just change my mind now in the last moment to make a Data a singletone, to avoid any risk of automatic failure.
I have the following question: What should happen if a caller of a singletone attempts to instanciate it with different file name?
Example:
1. Data.getInstance("c:\db.db");
2. Data.getInstance("d:\db.db");
Should I ignore it?

or should I check it?

Or should I just document the issue and use mutition as I do without any singletones?
Tx,
Vlad
 
Philippe Maquet
Bartender
Posts: 1872
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Vlad,
I'll dare now what I never dared before : asking you - with a red face - what C-tor means ...
Vlad, - and quite unfortunately - reading your post, I think of more questions than answers :

I have the following question: What should happen if a caller of a singletone attempts to instanciate it with different file name?
Example:
1. Data.getInstance("c:\db.db");
2. Data.getInstance("d:\db.db");

For me, those getInstance() calls are typically multiton. Where do you see any singleton ?

Should I ignore it?

code:
--------------------------------------------------------------------------------
// For lazy initialization public static synchronized Data getInstance(String fileName) { if (instance==null) { instance = new Data(fileName); } return instance; }
--------------------------------------------------------------------------------


That code doesn't make much sense, because the method signature is typically multiton, while the test you perform (instance == null) is a typical singleton test. I use a multiton design, but its names implies, instances are multiple (I store Data instances in a WeakHashMap) :

or should I check it?

code:
--------------------------------------------------------------------------------
// For lazy initialization public static synchronized Data getInstance(String fileName) { if (instance==null) { // SHOULD I DO IT??? if (!instance.fileName.equals(fileName)) throw new IllegalArgumentException("An instance of Data already" + exists, which contains another file name"); instance = new Data(fileName); } return instance; }
--------------------------------------------------------------------------------
And that code will throw a NullPointerException the first time instance will be null.
So please, I am willing to discuss it with you, but don't upload any of those versions !
Best,
Phil.
 
Rick Lu
Ranch Hand
Posts: 47
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

I did have Data class mutition, not a singletone. (My all remote connection factory creates a Data instance once and put the instance in a C-tor of RemoteObject every time).
Since I do synhronize all methods on "this", I am afraid that Sun's automatic test can attemp to create may istances of Data. My locking and synchronization will not work of course.

Currently, my design is very similar to yours. As my understanding, why not create a singleton LockManager for all Data instances and synchronize on it? IMO, Singleton Data may reduce the efficiency because too much model logic in it.
Rick
 
Vlad Rabkin
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Phil:
I'll dare now what I never dared before : asking you - with a red face - what C-tor means ...
Constructor
For me, those getInstance() calls are typically multiton. Where do you see any singleton ?
??? What is method for instanciating is used then typically for a singletone ???

I use a multiton design, but its names implies, instances are multiple (I store Data instances in a WeakHashMap)...

I hope it is not WeakHashMap where you implicetely manage your locks?

Ok I will try from the start:
1) I have seen that many people used Singletone. I cannot imagine how can be a singletone appllied for our assignement? Sorry it is a stupid question. May be I am tired, but if you have any idea could you show a sample how a singletone can be applied for our assignement? (I don't ask you if you like it, just a sample if you had used a singletone).
2) Shame on me. My understanding of multiton was totally wrong (see my previous mail).
Your sample for the multition looks great. It seems to me honestly saying the ONLY solution (As I said I can't imagine how a singletone can be applied to our assignement).
Just to make sure: dataInstances is a static class variable in Data class:

Is it right?
Sorry for all these stupid question, I am currently in two projects and I am totally tired
Tx,
Vlad
P.S. Phil, I owe you something: It seems that you prevented me from a very very big mistake!
[ October 14, 2003: Message edited by: Vlad Rabkin ]
 
Vlad Rabkin
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Rick,
why not create a singleton LockManager for all Data instances and synchronize on it?

It should be Ok for the assignement, but I PERSONALLY don't like this solution. I have explained it in previous threads: some operational systems have as a default limit on the number of open file connections. That is: if you have more than 256 client under the Solaris 5 (with standard config), you server would crash.
Best,
Vlad
 
Philippe Maquet
Bartender
Posts: 1872
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Vlad,

Phil:
I'll dare now what I never dared before : asking you - with a red face - what C-tor means ...
Constructor

Shame on me ! Anyway, after those few months passed to talk to each other, that stupid question will finally help me to understand you. So, it's OK !
For me, those getInstance() calls are typically multiton. Where do you see any singleton ?
??? What is method for instanciating is used then typically for a singletone ???

By nature, the getInstance() method of singletons take no argument. I meant that if you design a one-table system and if you want it to be a singleton, a simple getInstance() - I mean a getInstance(there_is_nothing_here) - is enough.
I use a multiton design, but its names implies, instances are multiple (I store Data instances in a WeakHashMap)...
I hope it is not WeakHashMap where you implicetely manage your locks?

Nothing to do with locking. The problem with a multiton (getInstance(some_parameter)), is that you offer the world a way to instantiate new objects (if they don't exist yet), without giving any way to drop them. Storing the instances in a WeakHashMap is just the way I found to get rid of that "disinstantiating" issue. It's just my way of doing and I never discussed it with others. (Andrew, Jim, others, do you agree with it ?).
2) Shame on me. My understanding of multiton was totally wrong (see my previous mail).
Your sample for the multition looks great. It seems to me honestly saying the ONLY solution (As I said I can't imagine how a singletone can be applied to our assignement).

Don't care. I know what getting tired of this assignment means ...

Just to make sure: dataInstances is a static class variable in Data class:

code:
--------------------------------------------------------------------------------
public class Data{static Map dataInstances = new WeakHashMap();... public static Data getInstance(String dbFileName) { }}
--------------------------------------------------------------------------------
Is it right?

Exactly that. Here is my own line of code :


Sorry for all these stupid question, I am currently in two projects and I am totally tired
Tx,
Vlad
P.S. Phil, I owe you something: It seems that you prevented me from a very very big mistake!

You're welcome. BTW, don't forget our promise : when we'll have passed that f..g assignment, we have to drink a few beers together on Brussels Grand-Place. I love drinking beers with friends so much, that if you had to fail, you'd get in big troubles with me (Frankfurt is not so far from Brussels) !
You are seeing the finishing line : don't hesitate to ask any last-question (I'll do the same soon). You did a so much great job here that we all are looking forward to see you pass with a high score ...
Best,
Phil.
 
Vlad Rabkin
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Phil,
Thank you so much for your kind words and great advice!
I have just tested it - works just great. I will have yet test if WeakHashMap really the Data instances withoout any special configuration, but I am sure it will work.
I have one additional question. I synchronize getInstance(fileName) not on dataInstances, but on "this" (because I synhcronize all on "this" and want to stick to the one concept overall. I am pretty sure it Ok to synhronize on this, but want anyway ask for your confirmation if it is Ok (No concurrency problem should arise):

Many Tx,
Vlad
[ October 14, 2003: Message edited by: Vlad Rabkin ]
 
Philippe Maquet
Bartender
Posts: 1872
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Vlad,
The code you posted above seems OK to me as far as synchronization is concerned. But be aware that your "synchronized" keyword in getInstance() (synchronize at the class level) has nothing to do (no relationship) with the "synchronized" keyword you use in your read() method (synchronize at the instance level). It's OK, but not related.
Now as you posted it, let me comment the code itself :
Forget the trim() on fileName, it's useless IMO. Or if you really want it, please trim() the fileName once in some String trimmedFileName local variable ! Such a way of coding really hurts me, should tire the JVM and ... could annoy a grader.
Sorry about my perfectionism, but I cannot read a piece of code without thinking of the CPU cycles it consumes.
Best,
Phil.
[ October 14, 2003: Message edited by: Philippe Maquet ]
 
Vlad Rabkin
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Phil,
But be aware that your "synchronized" keyword in getInstance() (synchronize at the class level) has nothing to do (no relationship) with the "synchronized" keyword you use in your read() method (synchronize at the instance level)

Yeap. I know. Agreed.
Forget the trim() on fileName, it's useless IMO
Such a way of coding really hurts me

Agreed.
Phil, I have the problem what I was afraid of:
WeakHashMap doesn't help to release the key/values.
I have tried manually put just some other key/values and call Syste.gc().
WeakHashMap realeases these key/values, but the ones put by getInstance():

The Sun says:
The value objects in a WeakHashMap are held by ordinary strong references. Thus care should be taken to ensure that value objects do not strongly refer to their own keys, either directly or indirectly, since that will prevent the keys from being discarded. Note that a value object may refer indirectly to its key via the WeakHashMap itself; that is, a value object may strongly refer to some other key object whose associated value object, in turn, strongly refers to the key of the first value object. One way to deal with this is to wrap values themselves within WeakReferences before inserting, as in: m.put(key, new WeakReference(value)), and then unwrapping upon each get.

My Data class has FileChannel instance variable:
Data.fileChannel -> RAF - > File -> fileName.
So, I have done what the Sun suggest:


It still doesn't help...
Have you tested it?
Do you know how should I handle it?
Tx,
Vlad
 
Philippe Maquet
Bartender
Posts: 1872
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Vlad,

Have you tested it?
Do you know how should I handle it?

Waouw ! Thanks a lot ! It's a bug, sorry !
No, I didn't test it (other uses of WeakHashMap well but not this one because it seemed so obvious - shame on me !).
The idea behind the scene here was to use a WeakHashMap to get automatically rid of Data instances when they are not in use anymore. The problem of the multiton design is that you store a strong reference in a HashMap, in such a way that your instance is never garbage-collected, even when the instance is not in use anymore.
But as the key in my implemtation is the filename (which is probably a local variable), you get rid off ... immediately !
I feel so grateful to you for having pointed it out, and in the same time so ashamed to have suggested to you a bug instead of a solution ...
Anyway, the least I can do now is to post a corrected version of Data.getInstance() - not tested :

Let me know if you have still an issue with it.
Sorry again,
Best,
Phil.
[ October 14, 2003: Message edited by: Philippe Maquet ]
 
Vlad Rabkin
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Phil,
But as the key in my implemtation is the filename (which is probably a local variable), you get rid off ... immediately !

Not really, there are some more confusing problems havinf String as a key:
This class is intended primarily for use with key objects whose equals methods test for object identity using the == operator. Once such a key is discarded it can never be recreated, so it is impossible to do a lookup of that key in a WeakHashMap at some later time and be surprised that its entry has been removed. This class will work perfectly well with key objects whose equals methods are not based upon object identity, such as String instances.

and in the same time so ashamed to have suggested to you a bug instead of a solution

Don't be too critical to yourself. My solution wasn't better
Ok, here is the final and tested version:

Phil, thank you so m-u-c-h!
It is a very elegant solution, but I am just a bit concerned if it is not too overcomplicated.
I am still wondering how could others use singletone for Data.
I guess that Jim has also multition. I know also that Andrew was considering a singletone.
Guys please, could you comment our discussion with Phil?
Best,
Vlad
 
Philippe Maquet
Bartender
Posts: 1872
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Vlad,
Ok, here is the final and tested version:

I love such a team spirit : I write, you test and point out a (big) bug, I debug, you retest and ... we get it !
It is a very elegant solution, but I am just a bit concerned if it is not too overcomplicated.

We have a french expression to describe such a situation : "You cannot get the butter and the money of the butter". Yes, it seems a bit complex but not over-complex IMO. After all, we are just discussing about a little 15 lines of code, curly braces included.

I guess that Jim has also multition. I know also that Andrew was considering a singletone.
Guys please, could you comment our discussion with Phil?

I'm interested too ! As I wrote above :
The problem with a multiton (getInstance(some_parameter)), is that you offer the world a way to instantiate new objects (if they don't exist yet), without giving any way to drop them. Storing the instances in a WeakHashMap is just the way I found to get rid of that "disinstantiating" issue. It's just my way of doing and I never discussed it with others. (Andrew, Jim, others, do you agree with it ?).

It seems OK to me (and Vlad just tested it), but any comment (or other solution) would be welcome.
Cheers,
Phil.
 
Philippe Maquet
Bartender
Posts: 1872
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Vlad,
I forgot to reply on this question :
I am still wondering how could others use singletone for Data.

People who use a Data singleton just make sure there is only one Data instance allocated (shared by all clients).
What we do with our multiton design is quite similar : we make sure that there is only one Data instance allocated per physical file.
In other words, the singleton pattern suits well for single-table db systems, while the multiton pattern allows your db system to deal with multiple tables.
BTW, before we go on, I'd like to come back to your first hesitation between singleton and multiton db designs, because reading again your first post, I wonder whether things are very clear for you about it :
Singleton is not "better" than Multiton as far as Data is concerned. Most people here (AFAIK) design a single-table system, in which case Singleton is perfect. A very few people here (but at least me) design a multi-table system, in which case Multiton pattern is mandatory. But the differences between both designs cannot be reduced to a simple choice between both patterns : to be consistent, choosing to implement a multi-table system requires that your locking scheme supports multiple table too. It seems obvious, and I cannot remember what you did for locking, but please make sure you're consistent with your own choices.
In fact, I do guess you'll get in trouble there. But if it's the case, please don't change your main db design choice : keep it single-table (it's very defendable), forget the multiton and replace it by a singleton. It will perfectly work while still consistent.
Please don't be mistaken about my purpose here : I know that you're finishing and that the worst I could do for you now would be to bring any confusion in your choices.
Well, the more I write in this post, the more I'd like - exceptionally - to borrow Mark's cap and say "Keep it simple, guy !".
Best,
Phil.
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks for the reminder, guys. Indeed, I'd made a mental note earlier to see about using a WeakHashMap for the multiton, but I neglected to actually put in a // TODO tag in my code (which Eclipse is nice enough to remind me about later) so I hadn't actually done it. Now, I have, and it seems to be working fine. I probably woud have been caught on the fact that only the keys are weakly linked, not the values, if not for the discussion here. Well, or if I'd tested it carefully; but I'm not sure I woud have thought to check carefully for GC of unused multitons, given that the class is really only used as a singleton under the GUI I've provided. So it seems this discussion was good for me.
Anyway, I used a WeakHashMap with File objects as keys, and WeakReferences as values, as suggested in the WeakHashMap API. My factory method looks like:

Looking at Vlad's code - ummm, why use a Map at all if you're just going to iterate through everything? Seems like you're using the Map backwards.
[ October 14, 2003: Message edited by: Jim Yingst ]
 
Philippe Maquet
Bartender
Posts: 1872
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Him,
Well, or if I'd tested it carefully; but I'm not sure I woud have thought to check carefully for GC of unused multitons, given that the class is really only used as a singleton under the GUI I've provided.

That's probably why I forgot to test it myself with gc ...
Anyway, I used a WeakHashMap with File objects as keys, and WeakReferences as values, as suggested in the WeakHashMap API. My factory method looks like:

Did you test it already, with File objects as keys ?
Looking at Vlad's code - ummm, why use a Map at all if you're just going to iterate through everything? Seems like you're using the Map backwards.

It's my code and I did it on purpose. Yesterday night, while testing a first version with dbFileNames as keys and data instances as values, Vlad noticed that gc made him loose the keys. At first sight, I thought that the dbFileName was not stored somewhere as a strong reference (in Data ?). So I suggested to invert the mapping the way I did. But now that we are early in the morning, I find myself that solution far less elegant than very late yesterday night ...
The problem encountered yesterday by Vlad maybe was caused by the fact that the key was a String ? Vlad, if you are reading me, I'll make some research and tests here before coming back with - hopefully - a workable and elegant final solution.
See you soon here.
Best,
Phil.
 
Vlad Rabkin
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Phil,
In fact, I do guess you'll get in trouble there. But if it's the case, please don't change your main db design choice : keep it single-table (it's very defendable), forget the multiton and replace it by a singleton.

That is what I am thinking all the time about. I have a single table design.
I don' understand how the singletone should work because we have Data.getInstance(String filename), not Data.getInstance() (How can I instance Data without knowing a filename?
So, Let's say I have Data.getInstance(String filename) as a singletone (not a mutliton).
Sun tester calles:
1. Data.getInstance("db.db");
2. Data.getInstance("the file which doesn't exist)
He will not get any exception. 2. will return him an instance of database connected to "db.db".
I could add a reference to the file name in Data class. I could each time also check if an instance exists (and if it exists if the file name is equal, otherwise throw an Exception).
That is actually what my first mail in the thread was about

Tx,
Vlad
 
Jim Yingst
Wanderer
Sheriff
Posts: 18671
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
[Philippe]: Did you test it already, with File objects as keys ?
Yes.
The problem encountered yesterday by Vlad maybe was caused by the fact that the key was a String ?
Possibly. Does the String appear as a String literal anywhere? E.g. in your test code? Then it's in the String pool, which is quite possibly (though not necessarily) a hard reference.
Though if you use WeakReferences for the values, the problem should go away. That is, the Map may still contain old keys and WeakReference objects which you don't need anymore, which aren't being GC'ed correctly, but those are probably small compared to your Data object. Well, they're definitely much smaller than my Data, since I use full caching. So for me, using WeakReferences is the key, and using WeakHashMap in addition to that just provides a little extra memory that can be freed up. If you want to have no caching, and plan for [i]lots of tables that take up very little memory, then perhaps the memory used by the keys and references is also significant, and the WeakHashMap is important.
On another topic:
[Vlad]: // SHOULD I DO IT???
I would, yes.
[ October 15, 2003: Message edited by: Jim Yingst ]
 
Philippe Maquet
Bartender
Posts: 1872
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Vlad and Jim,
There is so much to say, that I'll do in two separate posts.
The multiton issue first.
Jim:
[Philippe]: Did you test it already, with File objects as keys ?
Yes.

I did it too and I confirm (see test below).
Jim:
[Philippe]: The problem encountered yesterday by Vlad maybe was caused by the fact that the key was a String ?
Possibly. Does the String appear as a String literal anywhere? E.g. in your test code? Then it's in the String pool, which is quite possibly (though not necessarily) a hard reference.

Confirmed (I just tested it).
Jim:
Though if you use WeakReferences for the values, the problem should go away.

Yes, confirmed. Another solution (quite weird BTW but it works fine ) is to store Data instances as the WeakHashMap keys with null values. But you need to override hashCode() and equals() and the latter in a strange way (see below, it's funny).
Here are two workable solutions (I do prefer Data1) with their test :

Here is the test output :

Thanks a lot Vlad and Jim, you really helped me here !
Best,
Phil.
[ October 15, 2003: Message edited by: Philippe Maquet ]
[Andrew: Changed the in the code to ;) just to annoy Phil ]
[ October 15, 2003: Message edited by: Andrew Monkhouse ]
 
Philippe Maquet
Bartender
Posts: 1872
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Ah ah ! You'll have probably translated the nice smileys (this one ) in the middle of my code by a ';' followed by a ')'... Stupid software !
 
Philippe Maquet
Bartender
Posts: 1872
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Vlad and Jim,
Now about the singleton.

[Vlad]: // SHOULD I DO IT???
I would, yes.

Maybe yes, but not that way. I repeat that the first time getInstance() will be called, a NullPointerException will be thrown on line "if (!instance.fileName.equals(fileName))", but you'd notice it anyway at the first run.

Vlad, what I don't like too much with that solution is that if your DB admin needs to change the file name, he will have to restart the server.
Secondly, a singleton getInstance() method signature with parameter(s) looks weird IMO. Remember that's what confused me (sorry ) in your first post.
So what about this solution ?

Best,
Phil.
 
Vlad Rabkin
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Phil,
I like your solution Data1 very much. I bookmarked this topic, because I am sure I will use it in future for my own needs.
Phil, I have learned here two approaches:
1) Singletone throwing an exception if file name is not equal to the the one from existing instance of Data (if an instance already exists).
2) Multiton. It is very elegant a beatuful solution. You insist that this approach makes only sense if multitable design for db is applied.
I don't really agree with this statement. I have a single table approach. But, having a multiton allows me:
- correctly handle situation where different filenames are used by Data.getInstance()
- It allows me to have a remote server serving two different databases (not tables).
E.g. I han have RemoteConnectionFactory with two createMethods:
DBRemote Factory.createDB1(); // database 1 (file "db1.db")
DBRemote Factory.createDB2(); // database 2 (file "db2.db")

1) or you still beleive that in single table design multiton not needed?
2) Phil, It seems that Jim confirmed my that my version of singletone (thwoing IllegalArgumentEx) is Ok, but I still didn't understand your point regarding how would you implement a Singletone with getInstance(String s)?
Phil, Jim - many thanx,
Vlad
 
Philippe Maquet
Bartender
Posts: 1872
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Vlad,
1) Singletone throwing an exception if file name is not equal to the the one from existing instance of Data (if an instance already exists).

It's your original and current approach.

2) Multiton. It is very elegant a beatuful solution. You insist that this approach makes only sense if multitable design for db is applied.
I don't really agree with this statement. I have a single table approach. But, having a multiton allows me:
- correctly handle situation where different filenames are used by Data.getInstance()
- It allows me to have a remote server serving two different databases (not tables).
E.g. I han have RemoteConnectionFactory with two createMethods:
DBRemote Factory.createDB1(); // database 1 (file "db1.db")
DBRemote Factory.createDB2(); // database 2 (file "db2.db")

Good point. I agree.
1) or you still beleive that in single table design multiton not needed?

No. You just convinced me.
2) Phil, It seems that Jim confirmed my that my version of singletone (thwoing IllegalArgumentEx) is Ok, but I still didn't understand your point regarding how would you implement a Singletone with getInstance(String s)?

Well Jim didn't see my own reply yet, but I think he'll agree with me. If - despite what you just argued about the multiton design - you decide to use the singleton one, I still think it's better to have a Data no-arg private constructor with additional open() and close() methods.
Cheers,
Phil.
[ October 15, 2003: Message edited by: Philippe Maquet ]
 
Andrew Monkhouse
author and jackaroo
Marshal Commander
Pie
Posts: 12014
220
C++ Firefox Browser IntelliJ IDE Java Mac Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi guys,
The only reason for using a singleton is that it simplifies things right now at the expense of making things harder if a later enhancement required more than one table. But who ever heard of a database using more than one table? :roll:
Back when I was considering having a LockManager for my submission, I did have it as a multiton based on the database file name, however I had not gone so far as to put it in a WeakHashMap.
I have been looking at the iterators you have been using in your test cases, trying to see if they can throw ConcurrentModificationException. I think they could if someone added a new reference while you were printing the old instances. So if you were planning on leaving this debug method in your Data class, then you should probably fix that.
Regards, Andrew
 
Peter den Haan
author
Ranch Hand
Posts: 3252
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Sorry to butt in into this thread and maybe taking it into a (partial) tangent -- I believe Singleton to be an unequivocal mistake in this situation. In fact Singletons are a mistake in most situtations where people use them: they are the most abused pattern around.
A Singleton is appropriate when there is a fundamental reason, inherent in either your problem or your design, why there can only be one instance of a class. Using it for convenience when you happen to need just one instance of it is a mistake that makes your code inflexible, hard to extend, and less easy to understand (because the Singleton pattern sends out all the wrong signals to your reader). Is there a fundamental reason for the lock manager to be a Singleton? How many systems have you built that use exactly one database table? What does it mean for the evolution of a system if you would constrain it like that? How does the development time saved relate to this (although I doubt if any time is saved)?
For more information and extensive discussion, read Singleton Pattern (1), Singleton Pattern (2), Java Singleton Problems, Singletons are Evil.
- Peter
[ October 15, 2003: Message edited by: Peter den Haan ]
 
Vlad Rabkin
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Andrew,
I did have it as a multiton based on the database file name, however I had not gone so far as to put it in a WeakHashMap.

You talk here about LockManager. I ment Data class (I don't have any lockManagers as you know).
Andrew may I ask you the following question:
Did you allow multiple Data classes (I don't beileve it), did you make you Data class as Singleton or Multiton.
If the answer is Singletone how did you handle the problem:
Caller 1. Data.getInstance("db1.db");
Caller 2. Data.getInstance("db2.db");?
If the answer is Multiton:
Do you mean that you just used a standard collection (Map or some else) without taking any other care?
Tx,
Vlad
 
Vlad Rabkin
Ranch Hand
Posts: 555
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Peter,
I totally agree with you.
I don't need a singletone. My RemoteConnection Factory creates one and only one object of the Database in Constructor. RemoteObject Factory.create() return a new RemoteObject every time, but all of tham wrap the same Database object (Data).
My concern is that IF Sun doesn't use my Connectionfactory, but its own one, they can try to create each time a new Database instance (Data), which we leading my synchronitazion mechanism work improperly (I synchrnozae on this).
I hate singletones(multitons), but it seems to be reasonable to use it here
to avoid any risks.
Vlad
[ October 15, 2003: Message edited by: Vlad Rabkin ]
 
Philippe Maquet
Bartender
Posts: 1872
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Peter,
Knowing you by reputation, I confess that when I saw your post I thought "Waouw ! "The" Peter den Haan is joining us in our little SCJD thread !".
Thank you for the reference material on singletons : it was very interesting !
As my db design is multi-table, I use the multiton to manage Data instances as shown in Data1 class above. But as I use one singleton in my db package (LockManager), I now wonder if that use is OK or not ...
Andrew, your (just considered) LockManager class was a multiton :
Back when I was considering having a LockManager for my submission, I did have it as a multiton based on the database file name, however I had not gone so far as to put it in a WeakHashMap.

I mean that you had one LockManager instance per database file. I couldn't do that because I wanted that my deadlock detection (it's the main feature which justifies the whole class BTW) could prevent deadlock situations where multiple tables are concerned :
Client A is granted a lock on table1, recNo 100
Client B is granted a lock on table2, recno 200
Client A is claiming a lock on table2, recno 200
Client B is claiming a lock on table1, recno 100
As all locks are identified by table-recNo pairs, and as the LockManager main job must cover the whole database, it doesn't make any sense to let anyone instantiate anywhere another instance than the unique instance I expect. Even in the case I'd like to extend my current db design to support multiple databases (multiple multi-table-databases), it would be more beneficial to refactor LockManager to support multiple databases (changing the table-recNo pair in a database-table-recNo triplet can be done in 5 minutes), in order to extend the deadlock detection to cover the whole new multiple-databases system.
So LockManager in my design must be unique, period.
But reading Peter's post and his links to singleton references, I now really wonder whether singleton is suitable or not.
In my current design (single multi-tables database), at the Table (== Data) level, it's quite simple to enforce LockManager unicity by putting its lone instance in a class variable :

If I do it, the main benefit of this little refactoring is that I get rid off the singleton. (OK, I am kidding a bit, but if - by pure chance - Peter is my grader, it could be beneficial ).
But doing so brings a few cons : reading the LockManager class, a junior programmer (or even a senior one BTW) will probably wonder :
  • Is there one LockManager instance per Table instance (like in Andrew's first design) ?
  • I see there is a public constructor. Does it means that I may instantiate a LockManager ?
  • Does it make any sense to have multiple LockManager instances ?
  • If yes, in which context ?
  • ...


  • On the contrary, the singleton design is self-documented : any programmer reading the class sources will immediately conclude : OK, LockManager is unique, period.
    Better, if he cannot get that obvious conclusion by himself, the compiler will enforce its understanding.
    Now what would be the refactoring work in the case we extend the system to support multiple databases ?
    Common to both designs :
  • design a new Database class (mainly a collection of tables)
  • refactor Table to make it aware of the Database instance it belongs to
  • refactor LockManager to support the new database-table-recNo triplet
  • refactor lock() and unlock() Table methods for the same purpose


  • Additional refactoring work with the LockManager singleton design :
  • nothing


  • Additional refactoring work with the static LockManager instance stored at the table level :
  • move the static instance from Table to Database


  • Mmh... The more I discuss alone with myself , the more I think LockManager is a good example of a suitable singleton design.
    Peter, in case I'd get the extraordinary chance that you read this post, ... please comment it ! Thanks in advance.
    Jim:
    Though if you use WeakReferences for the values, the problem should go away. That is, the Map may still contain old keys and WeakReference objects which you don't need anymore, which aren't being GC'ed correctly, but those are probably small compared to your Data object. Well, they're definitely much smaller than my Data, since I use full caching. So for me, using WeakReferences is the key, and using WeakHashMap in addition to that just provides a little extra memory that can be freed up. If you want to have no caching, and plan for [i]lots of tables that take up very little memory, then perhaps the memory used by the keys and references is also significant, and the WeakHashMap is important.

    It cost me a second (or even third) read, but of course you are right as ... (I do hesitate between "so often" and "sometimes").
    Best,
    Phil.
     
    Jim Yingst
    Wanderer
    Sheriff
    Posts: 18671
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    [PM]: Maybe yes, but not that way. I repeat that the first time getInstance() will be called, a NullPointerException will be thrown on line "if (!instance.fileName.equals(fileName))", but you'd notice it anyway at the first run.
    Oops, agreed. To clarify, my position is that if you decide to use a singleton, and you use a getInstance() method that takes a file name as parameter, it would be a good idea to throw an IllegalArgumentException if the file is changed on a subsequent invocation.
    Is a singleton a good idea? Mmm, I'm not so opposed as PdH is. It can be useful to make sure no one's accidentally creating a second DB accessing the same file as the first, while also keeping things reasonably simple. However in this case I'd try to be clear in the documentation that while the current implementation may be as a singleton, a future implemenation may not be, and users should be careful about programming with the assumption that a singleton is used. This sort of approach can make sense as long as the API isn't being released to a public who will demand backward compatibility. E.g. it's not great for something like the java.lang classes, but can be OK for a small project where the code is only used internally.
    As it happens, implementing a multiton is not too complex here, IMO, but it's possible some designs might run into other complications.
    As for the getInsance() vs. getInstance(String) - that's arguable. If you're making a singleton which might become a multiton later, then getting callers to use getIstance(String) in the first place will make things a little easier later. So I disagree with Philippe in this respect, though I agree that the method looks like it's probably a multiton method, so it's a little confusing, and would need good documentation. If you're committed to having a singleton that will always be a singleton, then using getInstancce() makes more sense - but in this case, I more fully agree with PdH. Why would you want to rule out a multiton in the future, if it's simple to implement now?
    [PM]: Vlad, what I don't like too much with that solution is that if your DB admin needs to change the file name, he will have to restart the server.
    Well I'm not too concerned about that. Seems like there would be enough other issues with changing a file while the server is running, that this would add complexity and risk to handle smoothly. So my GUI design only accepts editing of the configuration on startup. If you want to change any paramenters, you have to stop the server and restart it. Maybe that's not the best design for a production server, but it keeps things simple for this assignment.
     
    Andrew Monkhouse
    author and jackaroo
    Marshal Commander
    Pie
    Posts: 12014
    220
    C++ Firefox Browser IntelliJ IDE Java Mac Oracle
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Peter,
    Good to see you posting in this forum again.
    In case you don't already have it, here's another link to an article talking about overuse of Singletons.
    Regards, Andrew
     
    Andrew Monkhouse
    author and jackaroo
    Marshal Commander
    Pie
    Posts: 12014
    220
    C++ Firefox Browser IntelliJ IDE Java Mac Oracle
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Vlad,
    You talk here about LockManager. I ment Data class (I don't have any lockManagers as you know).

    Sorry for the confusion. You had mentioned earlier that "I know also that Andrew was considering a singletone", and I wanted to make it clear that I do not think singletons are appropriate, hence my sarcasm with the "who ever heard of a database using more than one table?" and my example that I did consider multitons.
    Did you allow multiple Data classes (I don't beileve it), did you make you Data class as Singleton or Multiton.

    I did not make Data a Singleton or Multiton.
    Due to the way the starting code was presented to us by Sun, the Data class was neither a Singleton nor a Multiton - it was just a standard class that could have unlimited instances created. Changing that would have been a big mistake (IMHO) since that could potentially break any clients that already used the supplied Data class.
    If I had been writing the Data class from scratch I would probably have created it as a Multiton (which is what I was trying to say when I was talking about my LockManager).
    If the answer is Multiton:
    Do you mean that you just used a standard collection (Map or some else) without taking any other care?

    Again, because of the way the supplied Data class was written, I did not do anything special with tracking instances of Data class. It was up to the class that used the Data class to limit numbers of instances.
    Regards, Andrew
     
    Vitaly Zhuravlyov
    Greenhorn
    Posts: 16
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Vlad,
    I've just come back to my assignment and read this thread. I'd like to highlight the point that you should not rely on a file name but rather use a File object as Philippe does in Data1. Otherwise what happens if you invoke getInstance() method with "db-2x2.db" and "DB-2x2.DB" on Windows platform?
    Another my concern is applying the singleton or multiton patterns to the Data class directly. This means that you do not provide a public constructor for this class. On the other hand Sun explicitly specifies the name of the class and its package. Doesn't that mean that they are going to test this class automatically? And how can they instantiate it in this case? Tell me that I'm wrong.
    Thanks
    Vitaly
     
    Vlad Rabkin
    Ranch Hand
    Posts: 555
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Vitaly,
    you should not rely on a file name but rather use a File object

    Agreed.

    And how can they instantiate it in this case?

    First of all, I don't like Singletone/Multiton pattern. I have used Multiton to make sure, that Sun doesn't create more than one instance of Data per file.
    It is a 100% valid solution to provide getInstance() instead of public constructor for the Data class. Sun must either way create a Data object.
    They have to do it manually in both cases (public C-tor/getInstance()).
    Thus, I don't see a problem having Data.getInstance(String file).

    Best,
    Vlad
    [ October 28, 2003: Message edited by: Vlad Rabkin ]
     
    Vitaly Zhuravlyov
    Greenhorn
    Posts: 16
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Thanks Vlad. Although I do not agree that someone must manually specify a constuctor, the thing encouraging me is that there is no requirement for Data constructor in the instructions.
    Cheers
    Vitaly
     
    Vlad Rabkin
    Ranch Hand
    Posts: 555
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Vitaly,
    Although I do not agree that someone must manually specify a constuctor

    What ment is that Sun must use your C-tor (or getInstance()) to create your Data object. You must specify it.
    There is no way I would skip implementing the constructor of Data.
    The interface defined the functionality, which must be implemented. The interface never defines how the implementating class object must be created (C-tor). So, it is up to you, which C-tor or getInstance() method you provide. Sun has to accept it and modify its TestSuite to you individual C-tor/getInstance() of Data class.
    Best,
    Vlad
     
    Vitaly Zhuravlyov
    Greenhorn
    Posts: 16
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Agreed. Thanks Vlad.
     
    Wojtek Polczynski
    Greenhorn
    Posts: 6
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hello,
    I have just downloaded SCJD assignment and I have a question to you � especially to Phil.
    I would like to ask you about your Data / locking strategy. In this thread you wrote that you use the Multiton approach and there is just one instance of Data class per particular physical file. You also considered your locking strategy which has to work with multi-table solution. My design related to this part of our assignment is similar, apart from the locking functionality.

    I cannot understand why your variable called �lockManager� is declared as static. It means you have one instance of Lock Manager per JVM (per your application). Because your design allows to work with many tables you are forced to keep control over �record number � table name� pairs. I agree, this approach works fine but it is too complicated for me.

    I would like to have in my Data class an instance variable called lockedRecords. But I do not want to give it a static declaration. Therefore there is just one instance of lockedRecords per Data and there is just one Data per physical file. In this approach I do not have to keep �record number � file name pairs�. The only locked record number is sufficient information to store. Ooops! I did not mention I would like to synchronize inside lock() / unlock() methods on this instance variable - called lockedRecords.

    Could you please answer the following questions?
    - Is my data /locking approach correct?
    - If your answer is �no� please justify it.
    - If your answer is �yes� why have you chosen such a complex approach.

    Thanks in advance!
    Wojtek
     
    Philippe Maquet
    Bartender
    Posts: 1872
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Wojtek,
    Welcome to this forum.
    I cannot understand why your variable called �lockManager� is declared as static.

    If you read back my post above, it's not static, but LockManager is a singleton.
    It means you have one instance of Lock Manager per JVM (per your application). Because your design allows to work with many tables you are forced to keep control over �record number � table name� pairs.

    Yes.
    Could you please answer the following questions?
    - Is my data /locking approach correct?
    - If your answer is �no� please justify it.
    - If your answer is �yes� why have you chosen such a complex approach.

    My answer is "Yes!".
    I had to choose a slightly more complex approach because :
  • my locking scheme supports deadlock detection/prevention as explained above
  • as you, my db system is multi-table
  • I allow multiple locks to be granted to a given client, even if they span multiple tables


  • My deadlock detection/prevention system covers two features :
  • Automatic unlock of locks owned by crashed clients (within the next second)
  • Deadlock prevention as shown in the example above I repeat here :


  • Client A is granted a lock on table1, recNo 100
    Client B is granted a lock on table2, recno 200
    Client A is claiming a lock on table2, recno 200
    Client B is claiming a lock on table1, recno 100 --> Client B causes a deadlock
    In that case, Client B will receive a DeadLockException from its second lock() call and looses its first lock. As the situation must be much more complex than the one I just described (more than 2 clients / more than 2 locks envolved) my deadlock detection routine is recursive (thanks to Andrew ).
    I guess it's now clear why I need a central, unique, LockManager instance.
    Best,
    Phil.
     
    Wojtek Polczynski
    Greenhorn
    Posts: 6
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Phil,
    Thanks for your explanation. Now it is clear to me.
    I would like to ask you one more time.
    Could you please write an algorithm of your recursive method? It sounds really interesting to me.
    Thanks,
    Wojtek
     
    Philippe Maquet
    Bartender
    Posts: 1872
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Wojtek,
    I would like to ask you one more time.
    Could you please write an algorithm of your recursive method? It sounds really interesting to me.

    Sorry, but I think I cannot do it. If I had to post my deadlock detection routine alone (about 20 lines), you wouldn't understand it, because that private method relies on my whole LockManager architecture which you don't know and which is quite complex. And if I had to post my whole LockManager class, I'd break one of the main rules of this forum which is to avoid to post complete solutions.
    Sorry about that. Now, if you think of the problem by yourself, you should find a valid solution too which fits into your own design.
    Don't forget that deadlock detection is *not a requirement* and that very few people - AFAIK - implement it.
    Best,
    Phil.
     
    Wojtek Polczynski
    Greenhorn
    Posts: 6
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Thanks!
     
    • Post Reply
    • Bookmark Topic Watch Topic
    • New Topic