Singleton Pattern

[Mark110]

Ja ich weiss, dass viele von Euch das Pattern nicht mögen.
In meiner Situation finde ich es aber durchaus angebracht.


Meine Frage ist, ob ihr da etwa sanders machen würdet?
Die Umsetzung als Singleton sollte ich richtig gemacht haben?!

 

[tfa]

Mal davon abgesehen, dass dieses Pattern auch hier unangebracht ist, kann ich hier kein Singleton entdecken. Du kannst beliebig viele Objekte von OracleConnectionPool anlegen, oder was soll hier die Singleton-Klasse sein?

 

[Mark110]

naja ich kann doch immer nur eine instanz von ocpds erzeugen.

Hmmm stimmt es ist keine Instanz sondern nur eine variable... Ok es ist also kein Singleton... macht doch aber so wie ich es implementiert habe sinn?!

 

[tfa]

naja ich kann doch immer nur eine instanz von ocpds erzeugen.

Nein, dann müsste ocpds als Singleton implementiert sein, und das wäre schlecht.

Hmmm stimmt es ist keine Instanz sondern nur eine variable... Ok es ist also kein Singleton... macht doch aber so wie ich es implementiert habe sinn?!

Streich das Wort "Singleton" und nenn die Methode getDataSource().
Aber irgendwie sieht das nicht aus wie ein Connectionpool, sondern wie eine Factory bzw. Cache für ein OracleConnectionPoolDataSource-Objekt. Oder fehlt da noch was?

 

[byte]

Das hat eigentlich gar nichts mit dem Singleton-Pattern zu tun. Bei Bedarf können beliebig viele OracleConnectionPoolDataSource- und OracleConnectionPool-Instanzen erzeugt werden.

 

[Mark110]

Das es kein Singleton ist habe ich bereits eingesheen

Hmmm stimmt es ist keine Instanz sondern nur eine variable... Ok es ist also kein Singleton...

Fehlen tut da soweit nichts.
Ich habe zwei wietere Klassen in denen ich das folgende mache:

			dataSource = OracleConnectionPool.setupDataSource();
			
			// Create a pooled connection
		    pc = dataSource.getPooledConnection();
						
			conn = pc.getConnection();
			stmt = conn.createStatement();

ich wollte mir einfach nur doppelten code spaen, da ich das gleiche in zwei klassen machen muss.

 

[byte]

Und was ist nun Deine Frage? Ob das vom Design her schlecht ist? Dann lautet die Antwort: ja, da nicht objektorientiert.

 

[Mark110]

wie kann man das besser machen?

 

[xdavidx]

SP nutzt man doch nur bei Objekten Oo

- private Konstruktor
- private statische Referenz auf die Klasse
- statische Methode mit Prüfung, die die Referenz der Klasse "belegt" und returnt!

 

[xdavidx]

public class SingleTon {

	private static SingleTon instance;
	
	private SingleTon() {
		
		
	}
	
	public static SingleTon getInstance() {
		
		if(instance == null) {
		
			synchronized(SingleTon.class) {
				
				if(instance == null) instance = new SingleTon();
				
			}
			
		}
		
		return instance;
		
	}
}

Durch synchronized auch "Threadsicher"!

 

[tfa]

Durch synchronized auch "Threadsicher"!

Vorsicht: Das ist Double-Checked-Locking. Bis einschließlich Java 1.4 funktioniert das so nicht. Ab Java 5 klappt es, wenn man da noch ein volatile spendiert.
Es spricht nichts dagegen, die ganze Methode synchronized zu definieren. Dann hat man keine Probleme.

 

[maki]

Immer wieder diese Singletons mit lazy-initialisation, welches dann Threadsicher gemacht werden muss.

xdavidx, wozu die lazy-init?
Der ConnectionPool wird doch auf jedenfall gebraucht, also ist die lazy-init und die damit verbundene nötige synchronization doch für die Katz.

Wenn schon das (zurecht verrufene) Singleton Muster, dann doch bitte mit ein bisschen Sinn:

public class Singleton {

   private static Singleton instance = new Singleton();
   
   private Singleton() {}
   
   public static Singleton getInstance() {
      return instance;     
   }
}

 

[Wildcard]

Double-Checked-Locking finde ich immer wieder aufs Neue lustig. Ich Frage mich ernsthaft, wer (und vor allem warum) mit dem Pseudo Lazy Initializing überhaupt angefangen hat.

 

[Mark110]

SP nutzt man doch nur bei Objekten Oo

- private Konstruktor
- private statische Referenz auf die Klasse
- statische Methode mit Prüfung, die die Referenz der Klasse "belegt" und returnt!

Bis af den kosnturktor hatte ich das doch bereits



EDIT: es ist natürlich kein Singleton Pattern!!!
Es geht nur um die Umsetzung...

 

[byte]

Es spricht nichts dagegen, die ganze Methode synchronized zu definieren. Dann hat man keine Probleme.

Finde ich aber auch nicht so schön. Dann wird bei jedem Aufruf gelockt, selbst wenn das Singleton längst instanziert ist.

Am besten ist es imho, die Anwendung auf maximal ein Singleton zu beschränken, das nicht lazy initialisiert wird.

 

[maki]

Tombole,

du hattest keine Singleton, dachte das wäre klar?

Davon abgesehen ist das keine Vorraussetzung für ein Singleton:

- statische Methode mit Prüfung, die die Referenz der Klasse "belegt" und returnt!

Eine Methode die zB. getInstance heisst und diese eine Instanz der Klasse zurückgibt allerdings schon, und diese fehlte dir eben.

 

[tfa]

Es spricht nichts dagegen, die ganze Methode synchronized zu definieren. Dann hat man keine Probleme.

Finde ich aber auch nicht so schön. Dann wird bei jedem Aufruf gelockt, selbst wenn das Singleton längst instanziert ist.

Dieses Locking ist billig. Es sei denn, Du willst die Methode ein paar Millionen mal pro Sekunde aufrufen.

Am besten ist es imho, die Anwendung auf maximal ein Singleton zu beschränken, das nicht lazy initialisiert wird.

Mal abgesehen von der Singleton-Problematik finde ich an Lazy-Initialization nichts schlimmes. Natürlich nur, wenn die Objekterzeugung teuer ist. Lieber 1000 mal ein paar Millisekunden warten als einmal ein paar Sekunden.

 

[Wildcard]

Mal abgesehen von der Singleton-Problematik finde ich an Lazy-Initialization nichts schlimmes. Natürlich nur, wenn die Objekterzeugung teuer ist. Lieber 1000 mal ein paar Millisekunden warten als einmal ein paar Sekunden.

Nur macht es bei einem Singelton keinen Sinn, weil bei direkt Initialisierung die Initialisierung geschiet sobald die Klasse 'angefasst', bei Pseudo-Lazy sobald getInstance aufgerufen wird. Wann verwendet man die Singelton Klasse ohne gleichzeitig getInstance aufzurufen? Wozu soll das also gut sein?

 

[Mark110]

Irgendwie reden wir aneinander vorbei
Meine Frage nochmal: Ist meine Umsetzung so in ORdnung oder sollte das an der Stelle als Singleton umgestezt werden?
Ja jetzt ist es kein Singleton, aber meines erachtens erfüllt es dne zweck. Wenn es so nicht den Zweck erfüllt wäre es toll wenn Ihr mir sagen könntet warum es nicht den Zweck erfüllt.
Und mit Zweck meine ich auch die Berücksichtung eines guten Programmierstils.

 

[schalentier]

1. Du solltest den Logger an allen Stellen nutzen (und kein System.out.println)
2. Wenn das funktioniert ist doch gut, wenn du den Quellcode verbessern willst:
- static entfernen
- 2 Methoden drauss machen:
-- loadProperties()
-- createDataSource()
- Entscheiden fuer: Singleton oder Factory (wahlweise AbstractFactory oder FactoryMethod) und das entsprechend umsetzen

 

[byte]

Nur macht es bei einem Singelton keinen Sinn, weil bei direkt Initialisierung die Initialisierung geschiet sobald die Klasse 'angefasst', bei Pseudo-Lazy sobald getInstance aufgerufen wird.

Was meinst Du mit Pseudo-Lazy? Lazy-Initialisierung bedeutet, dass das Objekt erst dann initialisiert wird, wenn es gebraucht wird und nicht schon bei Programmstart. Das ist durchaus legitim. Bei größeren Systemen kannst du dadurch einen Slow-Start verhindern.

Wann verwendet man die Singelton Klasse ohne gleichzeitig getInstance aufzurufen? Wozu soll das also gut sein?

Wenn Du Module hast, die nicht zwangsläufig bei jeder Programmausführung verwendet werden?

 

[maki]

byto,

ist schon klar, wenn Lazy-init Sinn ergibt, sollte man sie auch verwenden.

Aber das ist bei weitem nicht immer der Fall, wage sogar zu behaupten, dass es in 90% der Fälle Sinnfrei ist, wird halt einfach so gemacht weil die Beispiele im Inet alle immer Lazy-Init verwenden, ohne zu erkären ob und warum diese nötig ist, bzw. was sie bringt.

Oder noch schlimmer, es wird angenommen, dass ein Singleton Lazy-init haben muss weil es sonst keines wäre..

 

[Mark110]

1. Du solltest den Logger an allen Stellen nutzen (und kein System.out.println)
2. Wenn das funktioniert ist doch gut, wenn du den Quellcode verbessern willst:
- static entfernen
- 2 Methoden drauss machen:
-- loadProperties()
-- createDataSource()
- Entscheiden fuer: Singleton oder Factory (wahlweise AbstractFactory oder FactoryMethod) und das entsprechend umsetzen

Danke für diese sinvollen Hinweise.
Leider verstehe ich nicht, warum ich ein singleton pattern nutzen soll in einer Klasse die OracleConnectionPool heißt und mit eine oracleConnectionpooldatasource geben soll.

Wenn ich in der Klasse eine Methode getInstance schriebe erhalte ich doch keine ocpds ?!
dann kann ich zwar in anderen klassen die klasse oracleconnectionpool instanziiren(einmal) und auf den wert zugreifen , z.b. durch eine get methode (instance.getOCPDS) aber ist es das was ich daraus machen soll?

Ich hoffe Eure geduld nicht zu sehr zu reizen :-(.

 

[maki]

Du hast eine static Factory.

Ich würde die Methode getConnection nennen und anstatt der Pooled DS eine Connection zurückgeben.
Dann müssen die Aufrufer nix von der Oracle spezifischen Implementierung der DS wissen.

 

[schalentier]

Zur Zeit hast du eine statische Methode in deiner Klasse. Statische Methoden sind oftmals nicht so dolle, denn die Wiederverwendbarkeit selbiger ist eingeschraenkt (kannst z.B. nicht ueberschreiben). Es ist halt nicht richtig objektorientiert.

Deshalb mach das static weg. Dann kannst du die Methode natuerlich nur an einer Instanz der Klasse aufrufen. Wenn du nun sicherstellen willst, dass es in deinem Programm nur genau eine Instanz der Klasse gibt, machst du ein Singleton. In diesem gibt es nur noch genau eine static Methode (getInstance) die dir die eine Instanz zurueckliefert. An dieser kannst du deine setupDataSource() aufrufen.

z.B.:

OracleDataSourcePool.getInstance().setupDataSource()

Jetzt koenntest du noch eine abstrakte Klasse DataSourcePool erstellen, OracleDataSourcePool von dieser ableiten und dort eine Methode loadProperties() hinpacken. Andere DataSourcePool-Implementierungen koennten diese Methode dann wiederverwenden. Das ist OOP.

Ob sich der scheinbare Mehraufwand in deinem konkreten Fall lohnt, musst du aber selbst entscheiden.

 

[Wildcard]

Nur macht es bei einem Singelton keinen Sinn, weil bei direkt Initialisierung die Initialisierung geschiet sobald die Klasse 'angefasst', bei Pseudo-Lazy sobald getInstance aufgerufen wird.

Was meinst Du mit Pseudo-Lazy? Lazy-Initialisierung bedeutet, dass das Objekt erst dann initialisiert wird, wenn es gebraucht wird und nicht schon bei Programmstart. Das ist durchaus legitim. Bei größeren Systemen kannst du dadurch einen Slow-Start verhindern.

Wann verwendet man die Singelton Klasse ohne gleichzeitig getInstance aufzurufen? Wozu soll das also gut sein?

Wenn Du Module hast, die nicht zwangsläufig bei jeder Programmausführung verwendet werden?

private static Singleton instance = new Singleton();

Diese Instanz wird nicht erzeugt, wenn die VM startet, sondern erst sobald Singleton auch verwendet wird. Es ist damit genauso Lazy wie die aufgeblasene, langsamere Pseudo Lazy Initializing Variante. Der einzige Unterschied tritt auf, wenn du Singleton.SOME_CONSTANT verwendest, ohne in absehbarer Zeit das erste mal Singleton.getInstance() aufzurufen.
Sind wir mal ehrlich, das hat keine Praxisrelevanz

 

[Mark110]

Zur Zeit hast du eine statische Methode in deiner Klasse. Statische Methoden sind oftmals nicht so dolle, denn die Wiederverwendbarkeit selbiger ist eingeschraenkt (kannst z.B. nicht ueberschreiben). Es ist halt nicht richtig objektorientiert.

Deshalb mach das static weg. Dann kannst du die Methode natuerlich nur an einer Instanz der Klasse aufrufen. Wenn du nun sicherstellen willst, dass es in deinem Programm nur genau eine Instanz der Klasse gibt, machst du ein Singleton. In diesem gibt es nur noch genau eine static Methode (getInstance) die dir die eine Instanz zurueckliefert. An dieser kannst du deine setupDataSource() aufrufen.

z.B.:

OracleDataSourcePool.getInstance().setupDataSource()

Jetzt koenntest du noch eine abstrakte Klasse DataSourcePool erstellen, OracleDataSourcePool von dieser ableiten und dort eine Methode loadProperties() hinpacken. Andere DataSourcePool-Implementierungen koennten diese Methode dann wiederverwenden. Das ist OOP.

Ob sich der scheinbare Mehraufwand in deinem konkreten Fall lohnt, musst du aber selbst entscheiden.

Vielen lieben dank!!

Ich habe nun eine letzte Version, in der ich hoffe die Tipps richtig umgesetzt zu haben.
Ein Feedback wäre also sehr nett.

import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.sql.Connection;
import java.sql.SQLException;
import java.util.Properties;

import javax.sql.PooledConnection;

import oracle.jdbc.pool.OracleConnectionPoolDataSource;

import org.apache.log4j.Logger;



public class OracleConnectionPool {

	
	private static final Logger log = Logger.getLogger(OracleConnectionPool.class);
	private static OracleConnectionPoolDataSource ocpds = null;
	private Properties prop = null;	
	private static OracleConnectionPool instance = new OracleConnectionPool();
	private Connection conn=null;
	
	private OracleConnectionPool() {
	}
	
	private void loadProperty() {
		// Load properties
		prop = new Properties();
		try {
			prop.load(new FileInputStream("db.properties"));
		} catch (FileNotFoundException e) {
			log.error("File not found Exception", e);
		} catch (IOException e) {
			log.error("Property IO Exception", e);
		}
	}
	
	public static OracleConnectionPool getInstance() {
		return instance;
	}
	
	public Connection setupDataSource() {
	
		if (ocpds == null) {
			instance.loadProperty();
			log.info("!null")			

			// Create a OracleConnectionPoolDataSource instance
			try {
				ocpds = new OracleConnectionPoolDataSource();
				// Set connection parameters
				ocpds.setURL(prop.getProperty("connectURL"));
				ocpds.setUser(prop.getProperty("user"));
				ocpds.setPassword(prop.getProperty("password"));
				PooledConnection pc = ocpds.getPooledConnection();
				Connection conn = pc.getConnection();
			} catch (SQLException e) {
				log.error("SQL Error", e);
			}
			return conn;
		} else {
			System.out.println("OracleConnectionPoolDataSource is != null");
			return conn;
		}
	}
}

 

[schalentier]

Nuja sieht ja schon mal ganz gut aus. Aber da is immer noch ein System.out dabei ;-)
Und in Zeile 59 legst du dir ne lokale Variable an, die es als Instanzvariable auch gibt. Das is vermutlich ein Bug.

Problematisch koennte jetzt noch das Exceptionhandling sein. Sollte irgendwas nicht klappen (Properyfile nicht gefunden oder DB Verbindungsprobleme), faengst du das zwar, aber deine Methoden liefern dann null zurueck.
Das hilft dir halt nicht viel. Sicherlich (ich kenn den Rest deiner Anwendung nicht) willst du das irgendwie dem Benutzer mitteilen, also koenntest du:
a) die Exceptions nicht fangen, sondern einfach weiterwerfen
b) die Exceptions fangen und eine eigne (z.B. DBConnectionException) werfen

und die woanders auswerten.

 

[Mark110]

das das die sysout meldungen sind ist nur der bequemlichkeit, da ich ncith ständig ins logfile schauen will sondern auch in der eclipse.
Die Zeile59 hast du nie gesehen Danke für dne Hinweis

Im endeffekt reicht es, wenn ich eine Exception im Log stehen hab, dann wird natürlich auch das Programm sterben, aber ich weiss dann warum.

Ich möchte mich ganz hrzlich bei dir bedanken. Ich habe das Singleton pattern jetzt tatsächlich verstanden

 

[Wildcard]

Schreib dir einen Log Appender der nach System.out schreibt, dann schlägst du 2 Fliegen mit einer Klappe. In der Auslieferung sollte dieser Appender allerdings ausgeschaltet sein.

 

[Mark110]

Danke für dne Hinweis Wildcard.
Der Appender existiert sogar bereits.

Hab ich nur nicht dran gedacht den einzuschlaten.

Danke

 

[byte]

private static Singleton instance = new Singleton();

Diese Instanz wird nicht erzeugt, wenn die VM startet, sondern erst sobald Singleton auch verwendet wird. Es ist damit genauso Lazy wie die aufgeblasene, langsamere Pseudo Lazy Initializing Variante. Der einzige Unterschied tritt auf, wenn du Singleton.SOME_CONSTANT verwendest, ohne in absehbarer Zeit das erste mal Singleton.getInstance() aufzurufen.
Sind wir mal ehrlich, das hat keine Praxisrelevanz

Stimmt, Du hast Recht!

 

[Saxony]

Hiho,

hier mal mein Standard-Link für jeden Singleton Thread.

bye Saxony

 

[schalentier]

Super Link, Danke.

 

[tfa]

In dem Artikel werden die Nachteile aber nicht so richtig angesprochen. Hier ist noch einer:
http://www.c2.com/cgi/wiki?SingletonsAreEvil

 

[xdavidx]

Hiho,

hier mal mein Standard-Link für jeden Singleton Thread.

bye Saxony

Hat mich überzeugt

Naja weniger Code is eh besser ^^