2016-11-30 4 views
6

Ist die unten aufgeführte Klasse ein Singleton? Da der Konstruktor als public deklariert ist, kann ich daraus schließen, dass die Klasse ein Singleton mit falscher Implementierung ist?Ist diese Klasse ein Singleton?

public class CreateDevice extends Functionality{ 

    private static Simulator simulator; 
    ConnectionDB connect = ConnectionDB.getInstance(); 


    public CreateDevice(Simulator simulator){ 
    this.simulator = simulator; 
    } 

    private static CreateDevice instance; 
    synchronized public static CreateDevice getInstance() { 
    if(instance == null){ 
     instance = new CreateDevice(simulator); 
    }  
     return instance; 
    } 
} 
+0

Eine Klasse kann kein Singleton sein, nur Objekte können Objekte sein, wenn sie einzigartig sind. Aber Klassen können das Singleton-Muster implementieren, um die Instanziierung auf ein Objekt zu beschränken :) – zapl

+0

Randbemerkung: 'simulator' sollte nicht' statisch' sein, wenn es im 'CreateDevice'-Konstruktor initialisiert wird - entweder statisch machen und über Klassennamen darauf verweisen 'CreateDevice.simulator' ODER entferne das' static' Schlüsselwort. (Das hat aber mit der Frage, Singleton zu sein, eigentlich nichts zu tun) – Hulk

+0

Sie haben recht. Dies ist ein problematischer Code, der geändert werden muss. –

Antwort

1

Sie können es zu einem Singleton machen, aber dann müssen Sie einen anderen Weg finden, den Simulator hinein zu injizieren.

In der aktuellen Implementierung wird der Simulator zum ersten Mal gesetzt, wenn jemand den Konstruktor aufruft. Dieser Konstruktor ist sehr seltsam, weil er ein statisches Feld setzt. Es wird auch sofort eine Verbindung getrennt von der Verbindung geöffnet, die von der Singleton-Instanz verwendet wird.

Wenn die Methode getInstance() aufgerufen wird, bevor der Konstruktor mindestens einmal aufgerufen wird, wird der Simulator nie festgelegt.

Um es zu einem richtigen Singleton zu machen, können Sie den Konstruktor entfernen und einen privaten No-Args-Konstruktor hinzufügen. Sie benötigen außerdem eine statische setSimulator() -Methode, die das statische Feld festlegt und sicherstellt, dass es aufgerufen wird, bevor eine andere Interaktion mit dem Simulator erforderlich ist.

Wenn Sie Abhängigkeiten zwischen Singletons haben, würde ich empfehlen, für das Inversion-of-Control-Muster zu gehen, wo ein IoC-Container die Service-Objekte erstellt und sie miteinander verdrahtet.

+0

Sie haben Recht, aber das Problem ist, ob ich es zu echten Singleton ändern sollte. –

+0

Wenn Sie es zu einem richtigen Singleton ändern, wird es eine Verbesserung gegenüber der aktuellen Situation sein. Aber die Verwendung von IoC wird noch mehr eine Verbesserung sein. – greyfairer

7

In einem Wort - nein. Da der Konstruktor public ist, kann jeder eine neue Instanz davon erstellen, wo und wann immer er will. Das Konstruieren des Konstruktors private sollte das Problem lösen und die Klasse in einen Singleton verwandeln.

+0

Die Sache ist der Code könnte falsch sein und ich schließe aus anderem Code, dass diese Klasse Singleton sein kann (Sie können sich vorstellen, dass die Funktion davon nur ein Gerät erstellt, so dass es nicht mehrere Instanzen benötigt) Also frage ich mich muss es in echten Singleton ändern. –

2

Ich würde sagen, Sie haben Recht, dass dies kein Singleton in der aktuellen Implementierung wäre.

Entweder müssen Sie den Konstruktor privat machen (was normalerweise gemacht wird), oder wenn Sie aus Legacy-Gründen einen öffentlichen Konstruktor benötigen, sollten Sie einen Konstruktor erstellen, der überprüft, ob die Instanz null ist und eine Ausnahme auslöst Wenn es bereits existiert oder es durch Aufruf eines anderen privaten Konstruktors erstellt, weisen Sie es der Instanz zu und geben Sie es zurück. Die erste Option ist vorzuziehen, während die andere für die meisten Projekte, bei denen ein Refaktor zu teuer ist, gut funktioniert.