I've been reading the SCJD Exam with J2SE book by Monkhouse & Camerlengo. I came across something that I don't understand and I was wondering what people thought?
They have DvdFileAccess class that controls access to the actual file where the data is stored, all instances of this class are meant to refer to the same file, and the constructor they have is outlined below.
The "if" statement seems odd - because it will always evaluate to true, so why have an "if" condition there in the first place. The database member variable is an instance variable, so when invoking the constructor the condition will always be true - the database variable will always be null at this point in the constructor.
So the code that creates a new RandomAccessFile will always be invoked when the constructor is called, and the "else if" portion of the code will never be executed. Unless I am misunderstanding something?
Hmmm...I was just double checking the text in the book that describes this code. I got the source code directly from the website. Strangely enough, the code in the book is different, and seems correct to me. The "if" condition is checking whether the dbPath is null (dbPath is a static variable), as oppossed to checking the whether the database is null (database is an instance variable) like the code associated with the book checks.
It seems to me that the source code associated with the book is incorrect, whereas the the code in the book itself is correct, it's on page 139 if anyone has the book. What do you guys think?
The constructor of the DvdFileAccess class as specified in the book:
The code from the book makes indeed more sense Andrew is a regular visitor of these forum, so maybe he might visit to comment on this one.
Maybe in the website source there is just a static keyword missing (to make database a class member). I agree that would not be very decent code, but maybe it's just to make you think about every line of code (instead of just blindly copying/pasting sources from the book). So I would say: mission accomplished Maybe it's just an old version of sources being downloadable from the website (and maybe it's mentioned in the errata page)
Yep, I think the code in the book is correct, and the code downloadable from the site is wrong. Two main reasons, firstly because if the downloadable code were correct, then the "if else" branch of the code would be unreachable, secondly because allowing each instance of the class to reset the location of the database wouuldn't make sense and would be dangerous.
I had checked the errata, but I couldn't find anything in relation to this.
Roel De Nijs wrote:maybe it's just to make you think about every line of code (instead of just blindly copying/pasting sources from the book). So I would say: mission accomplished
My impression is that this applies to a good bit of the code in that book. Some of it is overwrought. Some of it is too austere. Some of it is incorrect. I have assumed that all of this was intentional so that the book wouldn't simply dish up a complete serving of Finished Assignment™.
That's interesting . I can understand the overwrought and too austere, this is highlighted by the author at the start of the book, so I was taking any solutions with a pinch of salt, realising that I would need to question whether any solution was too much or too little based on my particular requirements.
But I don't think any author could claim that incorrect code\solutions were left in to make the reader think ! Fair enough in an exercise, but in the main text of the book, an incorrect solution merely distracts someone who notices it away from the main train of thought.
Thanks for the feedback guys. I think I understand where you are coming - basically don't copy the book for your assignment , am I correct?
My intention is to follow the book from beginning to end, understanding all the code, and noting down the main areas that need to be thought about with regards how the application is designed and implemented. I've also been reading this site which has mountains of information, so I'm noting down some thing I read in posts too. Then my intention is to then start off from scratch myself with my own design\implementation.
So all I was really doing here was questioning the code\design as I'm going through the book trying to understand "how does this work?", "is this a good design?", "is this a good implementation?".
On the thread you guys linked to, Andrew says himself that they wouldn't have deliberately added bugs to the code, which is reassuring to hear , so I think I will mail him on my findings and maybe he can update the codebase for the book.
Just to maybe give an idea of where I was coming from on this. My concern was around by allowing each instance of the DvdFileAccess class to reset the database location you could end up corrupting the file if someone was in the process of storing information to it - this book firstly checks the location of where to write a record, then at a later point synchronizes on the database (RandomAccessFile) to write to the file, so if another instance of the DvdFileAccess class was created with a different location in between these two actions, then the file would be corrupted.
Below is the method that is used to write to the file. But I'm happy I understand this now, and based on the code from the book my concern about the file being corrupted would not occur. Does this sound correct?
The book's purpose (I guess) is just to make you familiar with the concepts needed to pass this certification, not to copy all sources and submit your assignment.
As far as I understand the code your assumption is wrong. If a new instance of DvdFileAccess is created, a new RandomAccessFile instance (with different path) will be created, but the old instance will still have its own RandomAccessFile instance, because database is not declared static, so no corrupting of a database file.
Thanks Roel, I knew I was missing something! Yep, you are right, there would be no corruption of the database file regardless of which constructor was used (either the one from the book or the one from the source code).
On the purpose of the book. 100% agree, it should be viewed just as a reference. Which is what I am using it for. This thread was just to get some help\opinions as I was not quite sure if the code was correct or if something was amiss.
Alexandru Dragoi wrote:So in conclusion "database" variable must be static, isn't it?
That's not really required. Just 1 unique instance is required. One possible solution would be to make it static, but you can also ensure there is just 1 instance created of the class containing the database variable.
Joined: May 09, 2008
But the class that contains the database variable is "DvdFileAccess". Every client has its own instance of DvdFileAccess on the server side.
That is how the example in the book was presented. The author stated that the class should not be a singleton.
Positioning and reading/writing to the database file should be an atomic operation. That's why the book synchronizes on the database (instance) member. For creating/updating DVD records, the book definitely use 1 object which is the same for all threads, namely the recordNumbersLock (class) member. That's a very important difference to understand (and handle in your application).
Joined: May 09, 2008
Yes, it seems that I didn't take into account the fact that DvdFileAccess is a static member in DvdDatabase class.
So my affirmation:
Every client has its own instance of DvdFileAccess on the server side.
So in theory, it should be only one object that is used to access the database.
But in practice we can imagine this scenario:
1. Client1 is in the middle of reading a dvd from the database. For that it will use database object reference variable which refers to object DvdFileAccess1
2. In the same time Client2 connects to the server. It will update database static reference variable to point to a new object : DvdFileAccess2. Needless to say that this reference variable update has no effect on Client1 read opperation. Client1 still uses DvdFileAccess1 object to read from DB, because it did not even started reading (a context switch happened for example right before Client 1 was about to start reading).
3. Client 2 modifies a record in the database.
The problem that I see here is that DvdFileAccess1 and DvdFileAccess2 objects are using different locks to access the database (RandomAccessFile1 and RandomAccessFile2).
Reading/writing to the database file doesn't seem to be atomic.