Frage zu Connection.close()

[publicmain]

Hallo zusammen.

Ich möchte gerne eine Klasse bauen, mit der ich Daten in eine Datenbank persistent speichern kann. Dazu habe ich eine Klasse geschrieben, die eine java.sql.Connection hält. Man kann mit dieser Verbindung suchen, updaten und einfügen. Das funktioniert auch prima. Leider werden Daten aber nicht für immer in die Datenbank eingefügt, sondern nur wenn ich Connection.close() aufrufe, ansonsten nur so lange die Verbindung steht. Aber ich kann ja nicht nach jedem Insert einmal die Verbindung schließen und dann mit DriverManager.getConnection(...) wieder eine neue Verbindung machen. Oder ist das gängige Praxis? Es muss doch möglich sein, dass ich die Verbindung bis zum Schluss halte und wenn das Object zerstört wird auch die Connection geschlossen wird. Wie würde ich so etwas denn programmieren? Muss ich dafür extra eine Methode close() schreiben und diese aufrufen bevor ein Objekt Test zerstört wird?

public class Test {
	
	private Connection con = null;
	private PreparedStatement insertStatement = null;
	
	public Test()  {
		initConnection();
		initStatements();
	}
	
	private void initConnection()  {
		try {
			Class.forName("org.hsqldb.jdbcDriver");
			con = DriverManager.getConnection("jdbc:hsqldb:file:XYZ", "NAME", "PW");
			con.setAutoCommit(true);  
		} catch (Throwable e) {
			// ...
		}
	}
	
	private void initStatements()  {
		try {
			insertStatement = con.prepareStatement("INSERT INTO test.test (id, name) VALUES (?, ?)");
		} catch (SQLException e) {
			// ...
		}
	}

	public Status einfuegen (Test test) {
		Integer id = test.getId();  
		String name = test.getName(); 
		try {
			insertStatement.setInt(1, id);
			insertStatement.setString(2, name);
			insertStatement.executeUpdate();
			insertStatement.close();
			con.close();   // das möchte ich hier nicht
		} catch (SQLException e) {
			return Status.NICHT_OK
		}
		return Status.OK;
	}

}

Danke!

 

[dzim]

Es kommt darauf an... Ich nutze meist schon eine Connection nur innerhalb einer Methode, verliere dadurch sicher aber ein paar Vorteile, die man durch Prepared Statements erzielt.
Du kannst schon deine Verbindung in einer anderen Methode schliessen, aber du musst eben dann auch dafür sorgen, das die Verbindung geschlossen wird. Das Schliessen der Verbindung schliesst dir auch das PreparedStatement, wodurch du in deinem Code den Vorteil des PreparedStatement verlierst.

Als Verbesserungsvorschlag würde ich nur das Holen der Verbindung von der Klasse für die DB-Operationen trennen:

import java.io.File;
import java.sql.Connection;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.Locale;
import java.util.Properties;

import javax.sql.DataSource;

import org.apache.log4j.Logger;
import org.sqlite.SQLiteConfig;
import org.sqlite.javax.SQLiteConnectionPoolDataSource;

import ch.cnlab.performanceapplet.fx.db.DesktopDAOException;
import ch.cnlab.performanceapplet.fx.db.jdbc.definition.Tables;
import ch.cnlab.performanceapplet.fx.desktop.DesktopConstants;

public class ConnectionManager {
	
	private final static Logger LOGGER = Logger.getLogger(ConnectionManager.class.getName());
	
	private static SQLiteConnectionPoolDataSource dataSource = null;
	
	public static void init() throws DesktopDAOException {
		init(null);
	}
	
	public static void init(Properties props) throws DesktopDAOException {
		
		if (props != null) {
			// TODO check props and:
			// append to JDBC
			// or drop / update tables / ...
		}
		
		String userHome = System.getProperty("user.home");
		if (userHome == null || userHome.isEmpty()) {
			throw new DesktopDAOException("Could not determine the current user's home!");
		}
		
		SQLiteConfig config = new SQLiteConfig();
		dataSource = new SQLiteConnectionPoolDataSource(config);
		
		File dbPath = new File(userHome + DesktopConstants.USER_PATH);
		if (!dbPath.isDirectory()) {
			dbPath.mkdirs();
		}
		
		String url = String.format(Locale.ENGLISH, DesktopConstants.CONN_STRING_TEMPLATE, userHome);
		LOGGER.debug(String.format(Locale.ENGLISH, "JDBC-URL is: %s", url));
		
		dataSource.setUrl(url);
		dataSource.setDatabaseName(DesktopConstants.DB_NAME);
		
		try {
			initPerformanceDB();
		} catch (SQLException e) {
			throw new DesktopDAOException(e);
		}
	}
	
	public static DataSource getDataSource() throws DesktopDAOException {
		if (dataSource == null)
			init();
		return dataSource;
	}
	
	public static Connection getConnection() throws DesktopDAOException, SQLException {
		if (dataSource == null)
			init();
		return dataSource.getConnection();
	}
	
	private static void initPerformanceDB() throws DesktopDAOException, SQLException {
		
		Connection conn = getConnection();
		
		Statement stmt = conn.createStatement();
		stmt.executeUpdate(Tables.Preferences.SQL_CREATE);
		stmt.executeUpdate(Tables.Results.SQL_CREATE);
		stmt.executeUpdate(Tables.ResultDetails.SQL_CREATE);
	}
}

Hinweis: DesktopConstants, DesktopDAOException und Tables sind andere eigene Klassen.

Die Methoden #getDataSource() oder #getConnection() nutze ich dann, um diese dann an die Klassen weiterzureichen.
Beispielsweise hat die Implementierung meines (äusserst kreativen) DesktopDAO-Interfaces (der Name der Implementierung ist ebenso kreativ: DesktopDAOImpl) einen entsprechenden Konstruktor:

	public DesktopDAOImpl(DataSource dataSource) throws DesktopDAOException {
		if (dataSource == null) {
			throw new DesktopDAOException("No DataSource available!");
		}
		this.dataSource = dataSource;
	}

Wenn du das jetzt auf Connection abänderst, müsstest du nur noch eine #close()-Methode hinzufügen und dafür sorgen, das die Connection auch wieder ordentlich weggeputzt wird:

	public DesktopDAOImpl(Connection conn) throws DesktopDAOException {
		if (conn== null) {
			throw new DesktopDAOException("No Connection available!");
		}
		this.conn = conn;
	}

	// mein Interface hat diese Methode nicht
	// die könnte man hinzufügen, muss man aber nicht
	// denn egal wie: Hauptsache man ruft sie am Ende auf!
	public void close() {
		this.conn.close();
	}

 

[Joose]

Aber ich kann ja nicht nach jedem Insert einmal die Verbindung schließen und dann mit DriverManager.getConnection(...) wieder eine neue Verbindung machen.

Nein nach jedem INSERT bzw. anderem SQL Statement sollte die Connection nicht geschlossen werden müssen. Das Öffnen danach würde einfach zu lange dauern und die Performance stören!
Je nach Anwendung:

• wird ein ConnectionPool verwendet, so das es 40 DB Connections gibt und diese für >500 User dienen (Beispiel)
• Bei einer (Single)Client Anwendung wird die Connection erst geöffnet wenn gebraucht und sobald wie möglich wieder geschlossen. Sprich der User will einen Datensatz hinzufügen beim Bestätigen des Dialogs wird die Verbindung hergestellt alle INSERT, UPDATE und sonstige Statements werden durchgeführt danach wird die Verbindung geschlossen.

Zu deinem Problem:
Du hast zwar "autocommit" auf "true" gesetzt, aber bist du dir sicher das diese Einstellung zieht? So wie dein Problem beschrieben ist werden die Daten nicht commited (was beim Close automatisch passiert).

 

[publicmain]

Danke für eure Antworten!

Also ich möchte die Anwendung als Single-Client Anwendung laufen lassen, also nur für 1 Person und nicht für mehrere. Dann wäre es also sinnvoll die Verbindung immer wieder zu öffnen und zu schließen. Richtig?


Mein Problem ist nämlich folgendes. Ich möchte die Verbindung eigentlich am liebsten die ganze Zeit offen lassen, so lange der Client im Programm aktiv ist. Wenn er das Programm schließt, dann soll auch die Verbindung geschlossen werden. Mein Problem ist jetzt: Wenn der Anwender das Programm nicht ordentlich schließt, dann wird die Verbindung zur DB nie mit close() geschlossen und alle Änderungen wieder verworfen.
Gibt es auch für diesen Anwendungsfall eine gute Lösung?

 

[fehlerfinder]

Für eine Single-Client-Anwendung macht es Sinn, eine Verbindung beim Start deines Programms zu öffnen (denn dann wartet der Benutzer ohnehin ein wenig, bis ihm seine Oberfläche zur Verfügung steht) und diese beim Verlassen des Programms wieder zu schließen. Es ergibt überhaupt keinen Sinn, hier zwischendurch Verbindungen (Connections) auf- und abzubauen.

Ein Statement kannst du sicherlich nach Gebrauch so schnell wie möglich schließen, aber bei der Connection würde ich doch deutlich davon absehen.

Ich kenne die hsqldb nicht, habe gerade mal ein wenig herumexperimentiert, bin aber aus deinem Code für die Methode "einfuegen (Test test)" nicht schlau geworden. Wenn du ein ausführbares Programm zur Verfügung stellst, also z.B. noch eine zweite Klasse mit der main-Methode, die dann vermutlich direkt dein Test-Objekt erzeugen wird, will ich das gerne noch einmal nachvollziehen.

Ansonsten halte ich den Rat von Joose, zu prüfen, ob denn dein AutoCommit überhaupt funktioniert für eine gute Idee. Testweise könntest du setAutoCommit(true) auskommentieren UND im Anschluss an dein "executeUpdate(...)" ein Commit absetzen; wenn dadurch deine Daten dauerhaft in der DB gespeichert werden, bist du schon einen großen Schritt weiter ;-)

Du soltest schon beim Beenden deines Programmes die Connection vernünftig mit close() schließen und dies auch beim Exception-Handling berücksichtigen. Wenn der Benutzer dann sein Programm über den Task-Manager (Windows) oder per kill-Befehl (Linux) auf die harte Tour einfach abbricht, hat er's auch nicht anders verdient, als mit Datenverlust bestraft zu werden ;-) (nicht ganz ernst gemeint, aber doch mit einem Funken Wahrheit...)