Just as an FYI, I would argue that the InteruptedException should cause an explicit fatal exception to thrown: one that ripples up to middle tier as a FatalSystemException, and eventually to the user as a FatalGUIException, and which cause the system to shutdown. This is how most apps deal with this sort of thing in the real world.
M
Slightly more drastic than the assert. You are saying that the event should not have occured, and since it has occurred you do not believe your system should continue running at all.
If you want to do this, I would recommend you subclass RuntimeException, so you are throwing an exception that has a descriptive name so that any programmer can see what you are doing.
A second reason that a throwable may have a cause is that the method that throws it must conform to a general-purpose interface that does not permit the method to throw the cause directly. For example, suppose a persistent collection conforms to the Collection interface, and that its persistence is implemented atop java.io. Suppose the internals of the put method can throw an IOException. The implementation can communicate the details of the IOException to its caller while conforming to the Collection interface by wrapping the IOException in an appropriate unchecked exception. (The specification for the persistent collection should indicate that it is capable of throwing such exceptions.)
A cause can be associated with a throwable in two ways: via a constructor that takes the cause as an argument, or via the initCause(Throwable) method. New throwable classes that wish to allow causes to be associated with them should provide constructors that take a cause and delegate (perhaps indirectly) to one of the Throwable constructors that takes a cause. For example:
try {
lowLevelOp();
} catch (LowLevelException le) {
throw new HighLevelException(le); // Chaining-aware constructor
}
Because the initCause method is public, it allows a cause to be associated with any throwable, even a "legacy throwable" whose implementation predates the addition of the exception chaining mechanism to Throwable. For example:
try {
lowLevelOp();
} catch (LowLevelException le) {
throw (HighLevelException)
new HighLevelException().initCause(le); // Legacy constructor
}
SCJP,SCJD,SCWCD,SCBCD,SCDJWS,SCEA
Is this correct way of exception chaing for these kinds of exception.
[talking about exception chaining] Another exception that comes to my mind is the Magic Cookie value mismatch situation.
Should I be rethrowing the FatalSystemException as I show above?
I dare not write anymore, this will turn into a dissertation!
The Sun Certified Java Developer Exam with J2SE 5: paper version from Amazon, PDF from Apress, Online reference: Books 24x7 Personal blog
Hmmm - your code doesn't quite match up here. In your Data.lock() method, when you catch the InterruptedException, you are using the default constructor of FatalSystemException - you are not setting the cause. Yet later you try and get the cause from the thrown Exception (by the way - in your book() method, where you catch the FatalSystemException, you should check to see if whether you were able to retrieve the cause.
Hmmmm. You are actually wrapping the FatalSystemException inside another FatalSystemException. Is there any reason for this?
I can see value in both catching and rethrowing the FatalSystemException. Catching it lets the server deal with it (logging it, and/or shutting down). While rethrowing it lets it propogate up to the client, so that someone is aware of what the real problem is. The client can then scream to the sys admin that the server crashed.
By the way - I think your RecordNotFoundException in the book() method should probably be propogated in some way back to the client.
And I am not sure about throwing a RecordNotFoundException if the client already owns a lock.
SCJP,SCJD,SCWCD,SCBCD,SCDJWS,SCEA
SCJP,SCJD,SCWCD,SCBCD,SCDJWS,SCEA
Here is the modified code: [...] Is this OK?
[Andrew] By the way - I think your RecordNotFoundException in the book() method should probably be propogated in some way back to the client.
[Bharat] I agreee, I haven't yet worked out how should I do that. My initial thoughts:
1. Should I rethrow it with an appropriate added message string as I do above?
2. Should I throw a custom exception up the chain? - if yes then can you give me an example?
[Andrew] And I am not sure about throwing a RecordNotFoundException if the client already owns a lock.
[Bharat] By the way, is this another example of FatalSystemException type exception handling (I don't think so)
The Sun Certified Java Developer Exam with J2SE 5: paper version from Amazon, PDF from Apress, Online reference: Books 24x7 Personal blog
The chaining facility was designed to give more information to those clients that want to look at the cause. The way you are doing it forces someone to look at the cause. This is legal, but personally I think it would be nicer if you gave a descriptive message to go with it. So ...
throw new FatalSystemException("Invalid state", e);
//or
throw new FatalSystemException(e.getMessage(), e);
Personally I wouldn't even bother catching it. Just declare that your book() method throws the RecordNotFoundException, so if it occurs in your try block, it will automatically get propogated to the client.
I don't think you need to change the message received from your Data class, or chain it. The RecordNotFoundException is probably exactly what your client needs to receive.
I think we could argue that it is a FatalSystemException. After all, the clients we are developing should only ever try to gain on]e lock. If they try to gain more locks, then there are a whole bunch of issues that we have not catered for (and are not required in the specifications). So if they do try to get more than one lock, then they are going to cause the system to go into a state that has not been coded / tested.
However this may be a case where you want to catch the FatalSystemException in your book() method, determine that it occured because of attempting to lock more than one record, and just log it server side (don't shut server down) and then pass it up to the client. The server trapped it before it got into the invalid state, so you should be safe to carry on.
SCJP,SCJD,SCWCD,SCBCD,SCDJWS,SCEA
The Sun Certified Java Developer Exam with J2SE 5: paper version from Amazon, PDF from Apress, Online reference: Books 24x7 Personal blog
Now in your boolean book method, I don't see any other possible return value but true (false cannot be returned). Si this method could be a void one.
Why do you prevent clients to own more than one lock at a time
In your HashMap, recNo is the value. But as HashMap.containsValue() performs a sequential search, it may be very slow.
So, why don't you put recNos as keys and instanceRefs as values. If you allow multiple locks owned by a client, you wouldn't need to call containsValue() at all and your while() loop would be speed up by replacing the containsValue() by a containsKey().
Nate,
This part might have been overkill, depending on your design. If each client gets thier own thread(as in, say, a factory based RMI network approach), then you can use a WeakHashMap to store locked records in. As you know, the value of a WeakHashMap is that when a key object loses all references to it, it is released, as is the value object. The elegence of this approach comes in using the Thread itself as the key for the WeakhashMap, and the recordID as the value. This way, when a thread dies, it will eventually release the record. Thus, there's no need for a reaper thread.
HTH,
M, author
SCJP,SCJD,SCWCD,SCBCD,SCDJWS,SCEA
1. The assignment certainly doesn't prevent me from doing that does it? Further, it simplifies my work. - I can document it in my submission?
Which means that I cannot count on using a cookie value to unlock a locked record by the locking client.
In this thread Max quotes:
quote:
--------------------------------------------------------------------------------
Nate,
This part might have been overkill, depending on your design. If each client gets thier own thread(as in, say, a factory based RMI network approach), then you can use a WeakHashMap to store locked records in. As you know, the value of a WeakHashMap is that when a key object loses all references to it, it is released, as is the value object. The elegence of this approach comes in using the Thread itself as the key for the WeakhashMap, and the recordID as the value. This way, when a thread dies, it will eventually release the record. Thus, there's no need for a reaper thread.
HTH,
M, author
--------------------------------------------------------------------------------
If you read back your lock() code, you are waiting for a record to be available for locking : lockedRecords.wait(). When the WeakHashMap will be cleared of those entries belonging to a crashed client, it will be done silently, with no notification of any sort. You could solve the issue by replacing the wait() by a wait(timeout), but would it be in accordance with the instructions which state "the current thread gives up the CPU and consumes no CPU cycles until the record is unlocked" ? No, IMO. [B] I repeat that a solution based on WeakReferences registred with a ReferenceQueue and stored in regular HashMap is hardly more complex and ... works ! [B]
SCJP,SCJD,SCWCD,SCBCD,SCDJWS,SCEA
This sounds interesting. Can you give me some references/places I start looking?
Originally posted by Philippe Maquet:
Hi Bharat,
I hope I won't put some confusion in the thread here, and sorry if I do.
Here, Max surprises me ! The thread as the lock owner ? That's what I did in my first design and he criticized it just for that reason ?! I must miss something here.
[ September 09, 2003: Message edited by: Philippe Maquet ]
SCJP,SCJD,SCWCD,SCBCD,SCDJWS,SCEA
By coding and designing to support things that the client didn't ask for, it could be argued that you're not being strickly professionally ethical. That is, if this were a real project, I'm not sure you have the right to use the client's time and money in order to provide feature they didn't ask for.
This document deliberately leaves some issues unspecified, and some problems unraised. Your ability to think through these issues, in the face of realistically imperfect specifications, and come to a tenable solution is something upon which you are being graded.
Further, the requirements for this assignment don't require that you deal with client crashes, or other abnormal termination issues. To the point, they don't require that you deal with timeout of locks. Sun simply wants to know if you can write Thread safe code, as described in the requirements.
Put yourself in the grader's position for a minute. They want to see conventional solutions, they want easy-to-follow code, they want get done grading your assignment earlier then they had expected, and they want to go home.
Originally posted by Bharat Ruparel:
Hello Max,
Now I am confused! Are you saying that you were wrong in suggesting to store Data.this as the key for the static WeakHashMap and recNo as its value or you were wrong in your criticism?
If storing of "Data.this" as key is wrong then my locking class is incorrect! I don't think so because it seems to work, but I haven't tested it thoroughly yet.
Regards.
Bharat
Hi Max,
I agree with the main parts of your posts. But with these little nuances though :
quote:
--------------------------------------------------------------------------------
By coding and designing to support things that the client didn't ask for, it could be argued that you're not being strickly professionally ethical. That is, if this were a real project, I'm not sure you have the right to use the client's time and money in order to provide feature they didn't ask for.
--------------------------------------------------------------------------------
You are 100% right if this were a real project. But in such one, you would not have such a statement in your instructions :
quote:
--------------------------------------------------------------------------------
This document deliberately leaves some issues unspecified, and some problems unraised. Your ability to think through these issues, in the face of realistically imperfect specifications, and come to a tenable solution is something upon which you are being graded.
--------------------------------------------------------------------------------
I understand that they explicitly ask us to be reasonably creative to solve the issues they deliberately left unspecified ... and we are graded in that too. So it's not so easy IMO to know exactly where to stop in solving such unspecified issues.
quote:
--------------------------------------------------------------------------------
Further, the requirements for this assignment don't require that you deal with client crashes, or other abnormal termination issues. To the point, they don't require that you deal with timeout of locks. Sun simply wants to know if you can write Thread safe code, as described in the requirements.
--------------------------------------------------------------------------------
Agreed, with the same nuance. But anyway, if you do such additional things, they must work. I mean that if a static WeakHashMap specifically chosen as a solution to handle crashed clients don't work (see my comments in previous posts in this thread about the lack of notification), it's far better IMO use a regular HashMap instead and to "forget" the issue completely. As using a WeakHashMap there needs to be justified (in choices.txt), you cannot afford any unexpected wrong behaviour. Here we should agree totally : less is far better than wrong.
quote:
--------------------------------------------------------------------------------
Put yourself in the grader's position for a minute. They want to see conventional solutions, they want easy-to-follow code, they want get done grading your assignment earlier then they had expected, and they want to go home.
--------------------------------------------------------------------------------
That part of your post makes me afraid. Afraid to have written too much code based on too complex designs ... giving more job to the grader which could lead him to feel me as a quite antipathic guy. On the other hand, I suppose that graders don't read everything : I hope that after having read choices.txt, they choose parts of your source code which interest them more than other ones, in such a way that the global volume of your code would not be a real issue. Anyway I hope so, because if it's not the case, I am going straight in big troubles.
Best,
Phil.
Personally I don't think so. I believe that on application startup (both stand alone client and server) you should verify the cookie. Even if you do nothing else and then close the application. From then onwards you should be safe with cookie - the instructions tell you that no other application will modify the file while your application owns it.
So you could probably have the check either in the Data constructor (in which case you can throw any exception you like) or in your own method in the Data class (again you can throw any exception you like). You should not need to chain exceptions in this case.
(As an aside: You do (IMHO) want this check on application start up. There would be nothing worse than starting the server application, going home before anyone tries to do a booking, and later that the server crashed the first time someone tried to do the booking all because you had an invalid data file and it wasnt checked at startup. Much better to do a simple check at application startup and shut the server down instantly if the file is wrong.)
SCJP,SCJD,SCWCD,SCBCD,SCDJWS,SCEA
It's good to talk to again
It's a valid concern. OTOH, I'm sure you're not the first person to choose a socket implementation. And given how well you articulate you point on this forum, I'm sure you'll be fine
[Philippe] But it depends on how you interpret this sentence : "It must allow the user to book a selected record, ...". Does "a" means "one" for sure ? If you reply "yes", your design is OK ... for the moment. If you reply "no" or "maybe", your locking design is too restrictive.
[Philippe] the instructions that they intend to move the application to the web
[Philippe] I repeat that a solution based on WeakReferences registred with a ReferenceQueue and stored in regular HashMap is hardly more complex and ... works !
[Max to Phil] thanks for pitching in to help newer people as much as you have been. Andrew and I both appreciate it
[Bharat] (after throwing MagicCookieException) The only thing I am not sure about is that my console displays the stack trace for this exception while it displays the alert box. Since this is a show-stopper, I think it is OK to do so.
By the way, I am not sure at this point if it is advisable to pursue refining the locking stuff further based on Max's comments. I am inclined to leave it as it is provided that it is correct. What do you think?
The Sun Certified Java Developer Exam with J2SE 5: paper version from Amazon, PDF from Apress, Online reference: Books 24x7 Personal blog
Personally I do read the "a selected record" as one record. It is not "some records" or any other plural form.
True, but the instructions also tell us that "The IT director does not anticipate much reuse of the first Java technology system".
quote:
--------------------------------------------------------------------------------
[Philippe] I repeat that a solution based on WeakReferences registred with a ReferenceQueue and stored in regular HashMap is hardly more complex and ... works !
--------------------------------------------------------------------------------
But a ReferenceQueue still does not have any callback facility. So you still need some way of checking if anything is in the ReferenceQueue - possibly a separate thread.
In which case why not have a separate static variable containing the count of items that you have explicitly added / removed from your WeakHashMap? Your thread could periodically compare that number with the real number that is in the map, and if they differ, update the count and call notifyAll().
Your thread could periodically compare that number with the real number that is in the map, and if they differ, update the count and call notifyAll().
The only advantage the ReferenceQueue gives you is if you want to wake only those threads explicitly waiting for a specific lock.
We should remember your static locksCount solution for future posts on the same issue
The Sun Certified Java Developer Exam with J2SE 5: paper version from Amazon, PDF from Apress, Online reference: Books 24x7 Personal blog
If you had logging in your solution then you might want to send the stack trace to the logger. The client would then be able to send the log file to you so you can see exactly what went wrong. Much better than trying to work out what the client means when they phone you to say that "the application crashed because of some bad snack food".
Logging is not a requirement for this assignment, but it is very easy to implement. I have not yet deployed a live application that did not have some form of logging included.
SCJP,SCJD,SCWCD,SCBCD,SCDJWS,SCEA
Just as an FYI, I would argue that the InteruptedException should cause an explicit fatal exception to thrown: one that ripples up to middle tier as a FatalSystemException, and eventually to the user as a FatalGUIException, and which cause the system to shutdown. This is how most apps deal with this sort of thing in the real world.
Personally I don't think so. I believe that on application startup (both stand alone client and server) you should verify the cookie. Even if you do nothing else and then close the application. From then onwards you should be safe with cookie - the instructions tell you that no other application will modify the file while your application owns it.
So you could probably have the check either in the Data constructor (in which case you can throw any exception you like) or in your own method in the Data class (again you can throw any exception you like). You should not need to chain exceptions in this case.
Does it mean, that for example when I start the server for the networking mode, there must be a call to read the db.file in order to store the Metadatas in static members (either in primitive variables or within an own DBSchema-class)? So, when a Data-Class instance is created, the read MagicCookie is checked against the static value?
SCJP,SCJD,SCWCD,SCBCD,SCDJWS,SCEA
So I ripple a FatalError up ( how far is your own choice), log the error and exit the server. You could throw a Exception to the client if you wanted but you'd have to give the corrupted server some time to send it.
Any unimplemented exceptions in this interface must all be created as member classes of the suncertify.db package. Each must have a zero argument constructor and a second constructor that takes a String that serves as the exception's description.
Originally posted by Ulrich Heeger:
Should the server shutdown or only the client?
SCJP,SCJD,SCWCD,SCBCD,SCDJWS,SCEA
I think the server: The error happened on the server, and it affects every client. It's a legitimate time for a system crash, IMO.
SCJP,SCJD,SCWCD,SCBCD,SCDJWS,SCEA
How is it possible to just declare them in suncertify.db package level then?
Hello Max,
You wrote:
quote:
--------------------------------------------------------------------------------
I think the server: The error happened on the server, and it affects every client. It's a legitimate time for a system crash, IMO.
--------------------------------------------------------------------------------
How do you shut the server down?
Sounds fine, though I would probably use a finally clause
If the server goes, the client will get a RMI connection exception, which they can, in turn, treat as a TerribleException, etc.
Or, you could throw a TerribleException and start a Thread to shut down the server. Either works, though the latter can be messier.
In the standalone mode, when the networking mode is bypassed, can the InterruptedException occur? I would say no, because we don't have concurrent threads as long as I don't explicitly implement a second one. I'm right? So, I don't need to care about this Exception in this mode?
SCJP,SCJD,SCWCD,SCBCD,SCDJWS,SCEA
Originally posted by Ulrich Heeger:
Hi Max,
thanks for your reply, now I can go on
But I have just one more question.
1.How do you think the client should handle the TerribleException? I think, the client should get a message that his application will shut down, perhaps the current thread should sleep a while so the client can be aware of the message's content (and realize what's going on ) and after that the application shuts down.
What do you think about this idea?
2.In the standalone mode, when the networking mode is bypassed, can the InterruptedException occur? I would say no, because we don't have concurrent threads as long as I don't explicitly implement a second one. I'm right? So, I don't need to care about this Exception in this mode?
Thank you very much in advance,
Ulrich
but you don't need to sleep. Just pop up a message and let the client block until the customer presses ok. Then, do a system.exit.
SCJP,SCJD,SCWCD,SCBCD,SCDJWS,SCEA
They worship nothing. They say it's because nothing is worth fighting for. Like this tiny ad:
Gift giving made easy with the permaculture playing cards
https://coderanch.com/t/777758/Gift-giving-easy-permaculture-playing
|