Gleicher Code in Datenbankabfrage-Methoden reduzieren oder richtig so?

Kryptaesthesie

Kryptaesthesie

Erfahrenes Mitglied
Guten Morgen,

mich würde mal interessieren, wie ihr das macht.
Wenn ich eine Klasse für die Datenbankverbindung habe, dann habe ich da verschiedene Methoden mit unterschiedlichen Rückgabetypen.
Manche geben nur einen String, manche Integer, manche eine Liste, wenn mehrere Datensätze, usw.

Nun nutze ich auch einen Connection-Pool.
Würdet ihr das auch so machen, dass ihr in jeder Methode den gleichen Ablauf stehen habt: Connection, Statement, usw. holen und dann wieder schließen, usw.
Oder gliedert ihr sowas noch irgendwie aus?

Hier mal ein Beispiel einer der Methoden:
Code: 
    public String getString(String sql) {
        Connection conn = null;
        Statement stmt = null;
        ResultSet rset = null;
        
        String str = "";
        
        try  {
            conn = this.dataSource.getConnection();    // eine Verbindung aus dem Pool holen
            stmt = conn.createStatement();
            rset = stmt.executeQuery(sql);
            
            // gesuchten Wert auslesen
            if(rset.next()) {
              str = rset.getString(1);
            }
        } catch(Throwable t)  {
            logger.error("Fehler in getString('" + sql + "')", t);
        } finally {
            try { rset.close(); } catch(Exception e) { }
            try { stmt.close(); } catch(Exception e) { }
            try { conn.close(); } catch(Exception e) { }
        }
      
      return (str != null ? str : "");
    }
 
Mal davon abgesehen, dass Zeja völlig recht hat und man sowas einfach nicht selber codet (es sei denn zu Übungszwecken), ein paar Anmerkungen:

* bei den ganzen close Aufrufen solltest du vorher einen null check machen. Wenn die Exception nämlich beim aufmachen der Connection fliegt, sind Statement und ResultSet null
* ResultSet brauch man nicht explizit schließen, das wird vom Statement.close() mit geschlossen
* die Variable str kannst du dir sparen, ich würde in dem if-Block einfach return rset.getString(1); machen
* leere Strings zurückgeben ist foo...

Wiederverwendbarkeit ist ja hier auch so eine Sache. Du kannst im Endeffekt ja nicht beliebiges SQL ausführen sondern nur welches, das exakt einen String zurückgibt. Das Hauptproblem bei dem Design ist IMHO das Vermischen zwischen Ausführen des SQL Statements und dem Extrahieren der eigentlichen Daten aus dem ResultSet. Das JdbcTemplate von Spring bietet für letzteres einen RowMapper an (wenn es Typsicher (Java5) sein soll, nimmst du SimpleJdbcTemplate und ParameterizedRowMapper<T>).

Gruß
Ollie
 
Ich habe diese Klasse und das dahinter hängende Projekt vorgesetzt bekommen. Ich kann diese Klasse nicht einfach verwerfen, da das ganze Web-Projekt darauf zugreift.

Also meint ihr, es würde sich anbieten noch eine Schicht vor diese Klasse zu hängen, die die Daten aus der Datenbank holt und die jetzt aktuelle Klasse mit ihren Methoden so umzustricken, dass sie nur die entsprechenden Daten aus der neuen Klasse, die dann das Spring-Template verwendet, herauszieht?
So würde der Rest des Web-Projektes nicht mitbekommen, dass ich hinten was getan hat.

Danke schon mal für eure Hilfe! :)
Bin immer wieder sehr dankbar, wenn ich lerne, wie's schöner geht! ;-)
 
Die Klasse implementier hoffentlich ein Interface, richtig? Dann sollten Clientklassen ja nur das Interface kennen und du kannst bequem die Implementierung dahinter austauschen. Im Endeffekt brauchst du dir nur ein JdbcTemplate mit deiner DataSource instantiieren. Dann sollte eine der queryForObject Methoden ausreichen um deinen Stringparameter auszulesen.

Einen Schritt weiter gedacht ist man gut beraten pro Entität ein DAO einzuführen und das dann z.B. mit dem JdbcTemplate zu implementieren.

Gruß
Ollie
 
Oliver Gierke hat gesagt.:
Die Klasse implementier hoffentlich ein Interface, richtig?
Zum Vergrößern anklicken....
Natürlich nicht :(

Oliver Gierke hat gesagt.:
Einen Schritt weiter gedacht ist man gut beraten pro Entität ein DAO einzuführen und das dann z.B. mit dem JdbcTemplate zu implementieren.
Zum Vergrößern anklicken....
Die Idee dafür ist bis jetzt auf Grund von Zeitmangel nicht umgesetzt worden.
Außerdem ist im gesamten Projekt an den Stellen, wo Daten aus der DB benötigt wurden, direktes SQL implementiert. :(

Steinigt bitte nicht mich! Der Entwurf kommt nicht von mir! Ich muss mich nur leider jetzt damit rumschlagen.

Hier noch mal kurz der aktuelle Stand:
Eine Klasse Konstanten. In dieser Klasse steht folgendes:
Code: 
public static final DatenbankVerbindung DB = new DatenbankVerbindung("org.postgresql.Driver", "jdbc:postgresql", DB_SERVER + ":5432", "aktuelleDB", "user", "pw");
Wenn jetzt irgendwo eine Abfrage an die DB geschickt wird, sieht das in Etwa so aus (Beispiel für eine Zahl):
Code: 
Integer cnt = Konstanten.DB.getInteger("SELECT COUNT(*) FROM BEISPIEL");
Das komplett umzustrickt ist derzeit ausgeschlossen!
Darum meine Frage, ob es sich anbietet, dass ich eine Schicht hinzufügen sollte, die dann die eigentliche Verbindung zur Datenbank hat und die aktuelle DatenbankVerbindungs-Klasse nur nach außen hin noch genau so aussieht wie gehabt. Es sind einfach zu viele Stellen im Code, wo das geändert werden müsste. :(

Gruß
Gerrit
 
noch mehr so Dinger ... :-(

Guten Morgen!
Leute, ich brauche eure Hilfe!

Jetzt haben die hier das ResultSet aus der Datenbankverbindung rausgereicht - mit der Begründung: "Damit lässt sich doch jetzt prima arbeiten!".
Das gab fast Streit, weil ich immer wieder meinte, das will ich nicht und ich will da gefälligst ein Objekt drum rum, mit dem dann weitergearbeitet wird.

Mir gehen die Argumente aus! Was sind stichhaltige Punkte, die dagegen sprechen?
Das Argument, jetzt muss es auf Ewig eine Datenbank als Datenquelle bleiben wurde so beantwortet: "Was soll es denn auch sonst sein?".

Ein weiterer Punkt sind PreparedStatements. Da haben die Jungens vor langer Zeit mal eine Methode gebaut, die folgendermaßen aufgebaut ist:

  • PreparedStatement anlegen
  • SQL ausführen
  • Rückgabewert zurückgeben (für EINE SQL-Abfrage!)
Und jetzt wird diese Methode in Schleifen immer wieder aufgerufen. Aber das ist ja witzlos, weil das PreparedStatement-Objekt jedes mal aufs Neue angelegt wird.
Nachdem ich das angemerkt habe, wurde die Methode jetzt so umgeschrieben, dass das PreparedStatement in die Methode reingereicht wird :(

Alles in Allem sind jetzt Stellen im Code, wo außerhalb der Datenbankverbindung noch Statements und Connections geschlossen werden müssen. :( Wird mir schlecht, wenn ich sowas sehe und nichts dagegen machen kann ...
 
Das mit dem ResultSet ist mir schleierhaft. Das wird ja bekanntlichermaßen geschlossen, wenn das Statement geschlossen wird. D.h. um ein Resultset aus einer Methode rausreichen zu können darf man das Statement nicht schließen. D.h. wiederum du baust ein klassisches Memoryleak.

Desweiteren frage ich mich bei solchem Code (Konstanten.DB.getInteger("SELECT COUNT(*) FROM BEISPIEL");) wie man diesen Code testen will (alles statische Methoden) bzw., warum man dann überhaupt mit ner objektorientierten Sprache arbeitet, wenn man seinen Code so aufbaut, dass man von überall, an alles herankommt. Wenn man es überspitzt könntet ihr doch eigentlich komplett nur 1 Klasse machen und da 2000 statische Methoden reinwerfen: Willkommen in der prozeduralen Programmierung.

Architektonisch ist das ja auch so eine Sache. Wenn man an DB Connections von überall herankommt, verletzt sicherlich ein Haufen Code die Constraints von wo aus Datenbankverbindunen zu nutzen sind und von wo nicht. So, wie das momentan gestrickt ist, könnte man das ja sogar aus ner JSP heraus machen. Und ich bin mir sicher, dass sich Stellen im Code finden wo das passiert. Auf lange Sicht ist das ein Wartungsproblem. Mal abgesehen davon: wo ist denn dann noch eine Transaktionsgrenze wenn ih von überall auf die DB schießt. Die Frage ist ja auch, warum es denn überhaupt eine DB Abstraktion gibt, wenn dann viel Clientcode doch wieder mit den Lowlevel APIs arbeiten muss und die ganze Abstraktion hin ist.

IMHO zeigen solche Debatten eigentlich nur, dass man weder Ahnung von Softwaredesign und -architektur hat, noch Lust hat, sich damit zu beschäftigen. Sowas diskutier ich gern mal mit nem Studi, der bei uns nen Praktikum macht, aber wenn es mein Chef wäre, den ich vo sowas überzeugen müsste... ich weiß nicht, da würde ich mir Gedanken machen. Pragmatismus in allen Ehren, aber das ist einfach schlechter Code :).

Um jetzt ein wenig produktiv zu werden: nimm dir doch ein kleines Sück code, was mit der DB arbeiten muss und momentan die ganzen Sachen selbst handelt. Bau das exemplarisch nur für diesen Teil richtig (ein DAO z.B.) und zeig dadurch, wie der Clientcode ohne Datenbank *testbar* wird. Das ist er momentan nämlich unmöglich.
 
Leute die absolut unprofessionell arbeiten und der Meinung sind dass ein "läuft doch irgendwie" ausreicht trifft man leider immer wieder im IT-Bereich.

Einen guten Tipp wie man die Leute von solchem Verhalten wegbringt habe ich leider auch nicht. Könntest Artikel raussuchen wo Fachleute sagen, dass man das so nicht macht, allerdings glaube ich auch nicht dass sie darauf hören. Vielleicht mal mit dem Chef sprechen ob es möglich wäre mal jemanden für eine Schulung einzuladen.

Wenn die Leute irgendwann mal nicht da sind, aber du da bist Lock auf das Versionsverwaltungssystem und alles einmal ordentlich machen.

Argumente für das Thema mit dem ResultSet: Wer soll das close darauf aufrufen? Was passiert mit dem Statement, wer soll das schließen? Und wann? Irgendwann streikt einfach die Datenbank und keiner weiß woran es liegt. Ansonsten einfach nur "bäh". Absolut unsauberes Arbeiten ist das.
 
Ich habe auch gar kein Verständnis dafür! Das Ganze Projekt ist so undurchsichtig und Konstanten.DB.getString(sql) Aufrufe aus JSP heraus sind hunderte vorhanden! :(

zeja hat gesagt.:
Argumente für das Thema mit dem ResultSet: Wer soll das close darauf aufrufen? Was passiert mit dem Statement, wer soll das schließen? Und wann? Irgendwann streikt einfach die Datenbank und keiner weiß woran es liegt. Ansonsten einfach nur "bäh". Absolut unsauberes Arbeiten ist das.
Zum Vergrößern anklicken....
Anstelle des ResultSet direkt rauszureichen, wird ein Objekt rausgereicht, das folgendermaßen aussieht::
Code: 
package daten;

import java.sql.Connection;
import java.sql.ResultSet;
import java.sql.Statement;


public class XYZResultSet
{
    
    Connection conn = null;
    Statement stmt = null;
    ResultSet rset = null;
    
    /**
     * 
     * @param conn
     * @param stmt
     * @param rset
     */
    public XYZResultSet(Connection conn, Statement stmt, ResultSet rset)
    {
        this.conn = conn;
        this.stmt = stmt;
        this.rset = rset;
    }
    
    public ResultSet getResultSet()
    {
        return rset;
    }
    
    public void close()
    {
        // try { if(rset != null) rset.close(); } catch(Exception e) { } // Das ResultSet wird automatisch beim Schließen des Statements geschlossen!
        try { if(stmt != null) stmt.close(); } catch(Exception e) { }
        try { if(conn != null) conn.close(); } catch(Exception e) { }
    }
}
Habe kaum noch Haare auf dem Kopf - alle schon ausgerissen! :(

Wir bräuchten nicht nur eine Schulung, sondern einen erfahrenen Programmierer, bzw. eher einen Designer und nicht nur so "Frischlinge", wie ich auch einer bin.

Ich bin jetzt seit ca. 9 Monaten in diesem Team und habe wirklich schon jede Menge umgeschmissen und anders aufgebaut. Aber das war bis jetzt alles nur ein Tropfen Wasser auf einem heißen Stein.
Im Laufe der letzte Jahre ist das Ganze so dermaßen verwurstet worden, weil zu sämtlichen Anforderungen immer nur "Ja, können wir in der Zeit machen" gesagt wurde.
Habe gerade mal gesucht. Konstanten.DB.xyz() wird im Projekt an knapp 1000 (Tausend) Stellen und in allen Schichten angesprochen. :confused:

Nachtrag: nur so zur Info. Hier mal eine Stelle, an der das rausgereichte ResultSet verwendet wird:
Code: 
	public static void getRechnungPDF(OutputStream os, Integer auftraggeber, Calendar rechnungsdatum, String rechnungsnummer) {
		String sql = "";
		XYZResultSet xyzrs = null;
		try {			
			sql = "select rechnung from stammdaten.auftraggeber_rechnungen" +
									 " where datum = " + CalendarUtils.CalendarToSqlTimestamp(rechnungsdatum) +
									 " and rechnungsnummer = '" + rechnungsnummer + "'" +
									 " and auftraggeber = " + auftraggeber;
						
			xyzrs = Konstanten.DB.getStaticResultSet(sql);
			ResultSet rs = xyzrs.getResultSet();
			while(rs.next()) {
				InputStream in = rs.getBinaryStream("rechnung");
		    // create the byte array      
		    byte pic[]= new byte[in.available()];
		    // read the input stream
		    in.read(pic);
		    //und in den os schreiben
		    os.write(pic);
			}
		} catch(Exception e) {
			logger.error("Fehler beim Auslesen der Rechnung für #" + auftraggeber + " | SQL: " + sql);
		} finally {
			if(xyzrs != null) {
				xyzrs.close();
			}
		}
	}
 
Zuletzt bearbeitet:
Um antworten zu können musst du eingeloggt sein.

Neue Beiträge

Zurück