Question from Andrew's book

[Payal Shah]

In the book page 137, it says it is tempting to consider making the DVDFileAccess class a singleton (this class implements all the database access funtions) - coding the class in such a way that only one instance of DVDFileAccess can exist at any given time. ...
If we were to make the DVDFileAccess class a singleton, any class that uses the DvdFileAccess class would have to be coded differently than if it is a standard class --if we were to later decide that this same class can be used to process multiple data files(with some simple modifications) we would have to modify all the classes that use DvdFileAccess. There for this class is not a singleton...


I do not understand, how we have to changed the classes that use DvdFileAccess..

thanks

[Raza Khan]

Is it Andrews book... have you mentioned the correct page number

bye,
RazaKhan

[Payal Shah]

ya. it is Andrew's book. it starts at the bottom of page 137 to 138.

thanks

[Andrew Monkhouse]

A disclaimer for anyone reading this topic without having read the entire passage in the book: the coding differences is not the only reason for choosing not to go with a singleton, and certainly not the most compelling reason for not going with a singleton.

To answer your question Payal: Basically the standard way of dealing with any class from the client perspective may look something like:

DvdDataAccess myDataAccessor = new DvdFileAccess();

Whereas when dealing with a Singleton you usually have something like:

DvdDataAccess everybodiesDataAccessor = DvdFileAccess.getInstance();

It is hardly a huge difference, but if this is a commonly used class then you may have a large number of classes that you might need to modify if you were to change from a Singleton to a standard class.

Far more importantly is the concepts that go with it: ensuring that DvdFileAccess is then thread safe, ensuring that clients cannot use it in a non-thread safe manner.

Side note: it is possible to kludge a solution that does not require client changes - just have the getInstance method return a new instance every time. But this would be "a bad idea"™ as developers reading the client code might incorrectly think that they are dealing with a Singleton.

Regards, Andrew

[Payal Shah]

Andrew,

Thank you so much for your reply. I see your point now.

By the way great book and thank you so much for writing it.

Regards,
Payal

[Alecsandru Cocarla]

But, Andrew, since the DvdFileAccess instance is in fact "private static DvdFileAccess database" (in the code accompanying the book), what is the difference between a singleton and a static in this case? The "thread safe, ensuring that clients cannot use it in a non-thread safe manner" stuff applies in both cases anyway.

So why going against singleton, since in fact you're mimicking a singleton anyway in your own code??

[Andrew Monkhouse]

Hi Alecsandru

The difference between a Singleton class and the static usage of the class is in how they potentially can be used.

A Singleton class can only ever have a single instance within the context (in our case, within the JVM). (Unless, of course, you mis-code the Singleton ;)).

Defining a variable to be static only states that a particular class is potentially only going to use a single instance. It is still possible for other classes within the same context (JVM) to have their own instances, and indeed the other users can have multiple instances - they are not even limited to using static instances.

So I am not mimicking a Singleton in my code; I am simply declaring that the DvdDatabase class will only have a single instance of the DvdFileAccess class. Other classes may have their own DvdFileAccess class. Someone rewriting or improving my code can use it any way they feel like.

Regards, Andrew

[Alecsandru Cocarla]

So, at least in the current implementation available with the book, the DvdFileAccess is "thread safe, ensuring that clients cannot use it in a non-thread safe manner". And it's only a matter of calling new DvdFileAccess() instead of DvdFileAccess.getInstance() (and the potential benefit of having multiple instances sometime later).

In fact you're not using the same instance in all DvdDatabase instances, because the DvdDatabase constructor overwrites the instance every time it is called:

    /**
     * Constructor that takes the path of the Dvd database as a parameter.
     *
     * @param dbPath the path to the dvd_db directory
     * @throws FileNotFoundException if the database file cannot be found.
     * @throws IOException if the database file cannot be written to.
     */
    public DvdDatabase(String dbPath)
            throws FileNotFoundException, IOException {
        // Note: This is the only place we are acknowledging that we are using
        // a flat file instead of a commercial database. Create another Adapter
        // & replace this one line, & the system can use a commercial database.
        database = new DvdFileAccess(dbPath);
    }

The client might get in the strange situation of making an operation with one DvdFileAccess instance and, if another client already created a new DvdDatabase in the meanwhile, calling another one on some other instance of DvdFileAccess. This is not a problem in the current implementation (except the 5th issue I pointed out in https://coderanch.com/t/418338/Developer-Certification-SCJD/Errors-SCJD-Exam-with-JSE ). But this is, in my opinion, prone to errors if a programmer decides that DvdFileAccess should be "improved", making it stateful in any way (it's not a singleton, so it can have a state, isn't it? that's the whole idea of not having a singleton).

In my opinion, a singleton would have been much more appropriate in this case. You're trying (probably - see https://coderanch.com/t/418338/Developer-Certification-SCJD/Errors-SCJD-Exam-with-JSE no. 5) to have a single RandomAccessFile in the whole application anyway, so why using this construction which is convoluted, hard to understand and prone to (current and future) errors, when a singleton would be in fact the real concept behind all this stuff?

[Raza Khan]

Payal may i know what you do ? are you a btech or mca?

[Andrew Monkhouse]

Hi Alecsandru

You are correct in that there is a mistake in my code, and I have not correctly instantiated the DvdFileAccess class in my DvdDatabase.

A Singleton might be better for the particular example provided in the book. However the example in the book was contrived to be similar, but significantly different, to the real assignment. I strongly believe that using a Singleton for the real assignment is wrong, and I tried to make it obvious why this is the case in the book. If we had made the sample assignment closer to the real assignment then it would have been easier to demonstrate that using a Singleton in the book would have been wrong as well, however in doing so we would have given away the code for the real assignments Data class.

I'll try and look at your other topic a little later.

Regards, Andrew

[Alecsandru Cocarla]

Well, I used a singleton, not for the Data class, but for the file access manager class (and also for the record lock manager). And I think it's not wrong.

I think we'll just have to wait for the results (just uploaded my assignment) and see if this approach was wrong or not...

[Roel De Nijs]

Alecsandru Cocarla wrote:Well, I used a singleton, not for the Data class, but for the file access manager class (and also for the record lock manager). And I think it's not wrong.

And what is the reason why you made your file access manager thread safe? because you use synchronize on each method (read, update,...) to make them atomic operations?
And how did you implement your singleton, because i was also thinking about doing so and i was wondering if my singleton had to be thread-safe itself or not (i read some articles on the net about it and it's not that easy). And did you go for a classic implementation (with the getInstance) or for the new approach of joshua bloch: "a single-element enum type is the best way to implement a singleton"

public enum YourSingleton {   
    INSTANCE;   
  
    public void doStuff(String stuff) {   
        System.out.println("Doing " + stuff);   
    }   
}  

YourSingleton.INSTANCE.doStuff("some stuff");

[Andrew Monkhouse]

Hi Alecsandru,

Such is the beauty of software engineering: I can think that a Singleton is wrong for the SCJD assignment, and you can think it is right. And both of us can be right or wrong depending on a given perspective! As has been said before: give a problem to 2 engineers and you are likely to end up with 3 solutions.

Unfortunately we cannot prove either solution is absolutely right or wrong in this circumstance. Since there are no "must" requirements, you cannot get back an explicit failure if you get it wrong, nor will a pass mean that it is absolutely correct - at best a pass will only indicate that the solution you provide met the unknown requirements that the assessor was looking for.

If you are using a Singleton for the file access manager class, how are you going to handle any future enhancement requests (such as a likely request for the ability to handle more than one table within the application)? Or are you just considering this out of scope? (a valid design design).

Regards, Andrew

[Alecsandru Cocarla]

Andrew Monkhouse wrote:
If you are using a Singleton for the file access manager class, how are you going to handle any future enhancement requests (such as a likely request for the ability to handle more than one table within the application)? Or are you just considering this out of scope? (a valid design design).

For the assignment, I considered it out of scope.

In fact, with the current DB interface, there's no easy way out without a singleton. I'll consider that non-singleton means each Data class has its own RandomAccessFile. Or at least that every accessed file has its own RandomAccessFile. A single static RandomAccessFile does not solve the problem of different files being accessed (and, for only one file, it's just another way of doing it "singleton-like").

The current DB (the SUN interface) allows record-level locking only for update and delete. This means that, somehow, we have to lock also at read and create, because otherwise one Data instance might update a record, while another Data instance tries to read the same record, which results in reading a corrupted record. So, we need one unique shared mutex for every file. This mutex can be the RandomAccessFile itself, if we have one RAF per file, or a ReadWriteLock, or simply an Object to synchronize on.

This will result (in my opinion), again, in some kind of singleton - let's call it a "per-file-singleton": there must be only one instance of something (Data, FileAccessManager, RandomAccessFile, ReadWriteLock, Object) per file. I don't see any way out of this. Something has to be shared and unique per file, otherwise we can't synchronize access to that file.

What I could do with my current singleton approach:
- keep a static map of file access managers inside my FileAccessManager - (filePath - FileAccessManager) pairs
- change the FileAccessManager.getInstance() method to:

public static synchronized FileAccessManager getInstance(String filePath) {...}

- like the usual singleton, FileAccessManager should be non-cloneable, should have a private constructor and should be final.
- something similar for the record lock manager

That's all.

What I can conclude from this demonstration:
- even with multiple files, implementation is closer to some sort of singleton than to a real non-singleton
- in my opinion, there's no other way of doing this other than using this kind of "per-file-singleton"s

How would you do it? I mean, non-singleton way (we agreed that the implementation from the book is not really non-singleton).

[Alecsandru Cocarla]

Roel De Nijs wrote:
And what is the reason why you made your file access manager thread safe? because you use synchronize on each method (read, update,...) to make them atomic operations?
And how did you implement your singleton, because i was also thinking about doing so and i was wondering if my singleton had to be thread-safe itself or not (i read some articles on the net about it and it's not that easy). And did you go for a classic implementation (with the getInstance) or for the new approach of joshua bloch: "a single-element enum type is the best way to implement a singleton"

Synchronization is used only for file access operations. Any other operations (like transforming from string to byte) are performed asynchronous. So, the methods are not entirely synchronized, just portions of them.

I gave the reason why it has to be thread safe in my previous post - reads and updates must not happen at the same time over the same record.

I implemented the singleton the old way (I did not know about this new enum style). I don't really think it matters.
And I don't think that "single-element enum type is the best way to implement a singleton". If you need lazy-initialization, or if you need (like in the previous post) families of singletons, or if you simply don't want your singleton to have those enum specific methods, or if you think that enums should only be used for what they are meant (defining enumerations of some sort, not performing complex logic, file access and some other s**t), then you won't use an enum. I wouldn't.

[Roel De Nijs]

Alecsandru Cocarla wrote:
Synchronization is used only for file access operations. Any other operations (like transforming from string to byte) are performed asynchronous. So, the methods are not entirely synchronized, just portions of them.

Now I'm a little confused: you have 1 FileAccessMgr (the singleton) and one of your data-members will be a RAF. Instead of synchronizing the whole method you'll use a synchronized block in your method. Something like:

read() {
   // ...
   synchronized (raf) {
      raf.seek(position);
      raf.read(record);
   }
   // ...
}
update() {
   // ...
   synchronized (raf) {
      raf.seek(position);
      raf.write(updatedRecord);
   }
   // ...
}

Is this the thread-safe approach you're using?

If you make the RAF in the FileAccessMgr static, I would say (but it's already late in the evening) your FileAccessMgr is also thread-safe and you can have as many FileAccessMgr-instances as you want. Or do I miss something here? If so, please enlighten me.

Alecsandru Cocarla wrote:
And I don't think that "single-element enum type is the best way to implement a singleton".

I agree with you. For those who want to read more about enum and singleton: Josh Block's JavaOne 2008 More Effective Java (pdf) (p. 29-31)

[Alecsandru Cocarla]

Andrew, did you get to see my post about the singleton implementation of multiple files?
My question to you is still waiting :

Alecsandru Cocarla wrote:How would you do it? I mean, non-singleton way (we agreed that the implementation from the book is not really non-singleton).