0

I want that only one instance of my class, Format1 should be returned through the class Engine: 

 public final class Engine {

     private static Format format;

     public Engine(final Param1 param1, final Param2 param2) {
         Engine.format = new FormatV1(param1);   // FormatV1 implements Format
     }

     static Format getFormat(int condition) {
         switch(condition) {
            case 1 : return format;
         }
     }
 }

Does this ensure that only one and correct instance of Format is returned via getFormat?

Also, the client which invokes the getFormat() needs to execute method on the format object (which has absolutely no state) but passes a ByteBuffer to the method (this ByteBuffer might be modified):

 class Client 
 {
     public static void main(String[] args) {

          Format obj = Engine.getFormat(1); // some param
          ByteBuffer buffer;                // read data from disk

          synchronized (obj) {

                 long[] result = obj.method(buffer);

                 if (result == null) {
                      // do something
                 }
          }
     }
 }

Does the synchronized construct here/like this ensure serializability ?

P.S: I'm very new to Java, as much as I read I could also make the method synchronized but I think the check (result == null) should also be included in the critical section so I decided to use the synchronized (obj) construct instead. Please forgive me if this is a silly question but I want to confirm my doubts.

user1071840
  • 3,522
  • 9
  • 48
  • 74
  • Actually your method `getFormat` is based on a condition, so it won't return a singleton unless you always use the same parameter (in this case `1`) – Omar Mainegra Sep 11 '13 at 18:14
  • 1
    Singleton class should not have a public constructor. – PM 77-1 Sep 11 '13 at 18:18
  • I understand and I'll always be sending the same parameter. Also, I know this isn't exactly a singleton..but all I want is that everytime the BTSEngine gets a request same object should be returned. – user1071840 Sep 11 '13 at 18:25

2 Answers2

0

currently, you are creating the Format instance in the Engine constructor, which is most likely not what you want (your current example would throw a NullPointerException.

secondly, you have 2 synchronization concerns. first is the correct publication of the format instance. you don't do this currently. you should either use class initialization (instantiate the format instance on class create) or make the format member volatile in order to correctly publish the format instance.

secondly, if your format instance has mutable state, then you should make the FormatV1 methods themselves synchronized. making the caller synchronize on the object before using it is a very bad design.

jtahlborn
  • 52,909
  • 5
  • 76
  • 118
  • If I make the method synchronized..what about the test condiiton ..that needs to be within some sync block too..how to achieve that? – user1071840 Sep 11 '13 at 18:28
  • I don't understand, "making the caller synchronize on the object before using it"..Isn't that mostly what synchronized(object) does? – user1071840 Sep 11 '13 at 18:33
  • @user1071840 - what you are doing is called "client side locking" which subverts object encapsulation (for instance, the FormatV1 impl could use something _besides_ standard synchronization to handle thread-safety). why do you need the test condition to be within the sync block? – jtahlborn Sep 11 '13 at 18:38
  • oh..yeah..no 2 threads can enter the synchronized method and see inconsistent result..yeah, will use synchronzied methods. – user1071840 Sep 11 '13 at 19:09
0

You created a constructor of the Engine class to initialize the static variable, instead, create a static initializer, using synchronized on that method

public static synchronized void initFormat(....) {
   if (format == null) {
      ... create it
   } else {
   .... your chocie: throw an exception or do nothing 
   }
}
GerritCap
  • 1,606
  • 10
  • 9