2 ComboBox(en)

[maGG]

So ich hab mal rumprobiert und zu mindest ist nix mehr rot :'D

package RepositoryClasses;

import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.EnumMap;
import java.util.List;
import ObjectClasses.Branch;

public class BranchRepository{
  
    public enum Location{
        DOMESTIC, FOREIGN,
        DOMESTIC_UPDATE, FOREIGN_UPDATE,
        DOMESTIC_INSERT,FOREIGN_INSERT
    }
  
    private static final EnumMap<Location, String> queries = new EnumMap<>(Location.class);
  
    static{
        queries.put(Location.DOMESTIC, "SELECT * FROM Filialen_Deutschland");
        queries.put(Location.FOREIGN, "SELECT * FROM Filialen_Ausland");
        queries.put(Location.DOMESTIC_UPDATE, "UPDATE Filialen_Deutschland SET id=?, LABEL=?, STRASZE=?, PLZ_ORT=?, ORT=?,TEL_LAND=?, " +
                "TEL_ANFANG_INT=?, TEL_ANFANG_TXT=?, TEL_ENDE_DEFAULT=?, FAX_ANFANG_TXT=?, FAX_ENDE_TXT=? WHERE id=?");
        queries.put(Location.FOREIGN_UPDATE, "UPDATE Filialen_Ausland SET id=?, LABEL=?, STRASZE=?, PLZ_ORT=?, ORT=?,TEL_LAND=?, " +
                "TEL_ANFANG_INT=?, TEL_ANFANG_TXT=?, TEL_ENDE_DEFAULT=?, FAX_ANFANG_TXT=?, FAX_ENDE_TXT=? WHERE id=?");
        queries.put(Location.DOMESTIC_INSERT, "INSERT INTO Filialen_Deutschland (id,LABEL,STRASZE,PLZ_ORT,ORT,TEL_LAND,TEL_ANFANG_INT," +
                "TEL_ANFANG_TXT,TEL_ENDE_DEFAULT,FAX_ANFANG_TXT,FAX_ENDE_TXT) VALUES(?,?,?,?,?,?,?,?,?,?,?)");
        queries.put(Location.DOMESTIC_INSERT, "INSERT INTO Filialen_Deutschland (id,LABEL,STRASZE,PLZ_ORT,ORT,TEL_LAND,TEL_ANFANG_INT," +
                "TEL_ANFANG_TXT,TEL_ENDE_DEFAULT,FAX_ANFANG_TXT,FAX_ENDE_TXT) VALUES(?,?,?,?,?,?,?,?,?,?,?)");
    }
  
    protected Branch branch;
    private Connection conn;
  
    public BranchRepository(){}
  
    public BranchRepository (Connection conn){
        this.conn = conn;
              
    }  
  
    public Branch findNames(Location where) throws SQLException{
        String sql = queries.get(where);
        try(PreparedStatement pst = conn.prepareStatement(sql)){
            try(ResultSet rs = pst.executeQuery()){
                branch = new Branch();
                while(rs.next()){
                    branch.setLabel((rs.getString("Label")));
                }
                return branch;            
            }
        }
    }  
  

    public void updateBranch(Location where, int f1, String f2, String f3, String f4, String f5, int f6, long f7, String f8,
            String f9, String f10, String f11) throws SQLException{
        String sql = queries.get(where);
        try (PreparedStatement pst = conn.prepareStatement(sql)) {
            branch = new Branch(f1, f2, f3, f4, f5, f6, f7, f8, f9, f10, f11);
            pst.setInt(1, branch.getId()); //id
            pst.setString(2, branch.getLabel()); //LABEL
            pst.setString(3, branch.getStrasze()); //STRASZE
            pst.setString(4, branch.getPlzort()); //PLZ_ORT
            pst.setString(5, branch.getOrt()); //ORT
            pst.setInt(6, branch.getVorwahl()); //TEL_LAND
            pst.setLong(7, branch.getTelStartNum()); //TEL_ANFANG_INT
            pst.setString(8, branch.getFaxStartTxt()); //TEL_ANFANG_TXT
            if (branch.getTelEndDefault().equals("")) {
                pst.setObject(9, null);
            } else {
                pst.setInt(9, Integer.parseInt(branch.getTelEndDefault())); //TEL_ENDE_DEFAULT
            }
            pst.setString(10, branch.getFaxStartTxt()); //FAX_ANFANG_TXT
            if (branch.getFaxEndTxt().equals("")) {
                pst.setObject(11, null);
            } else {
                pst.setString(11, branch.getFaxEndTxt()); //FAX_ENDE_TXT
            }
            pst.setInt(12, branch.getId()); //id
            pst.executeUpdate();
        }  
    }

    public void insertBranch(Location where, int f1, String f2, String f3, String f4, String f5, int f6, long f7, String f8,
            String f9, String f10, String f11) throws SQLException{
        String sql = queries.get(where);
        try (PreparedStatement pst = conn.prepareStatement(sql)) {
            branch = new Branch(f1, f2, f3, f4, f5, f6, f7, f8, f9, f10, f11);
            pst.setInt(1, branch.getId()); //id
            pst.setString(2, branch.getLabel()); //LABEL
            pst.setString(3, branch.getStrasze()); //STRASZE
            pst.setString(4, branch.getPlzort()); //PLZ_ORT
            pst.setString(5, branch.getOrt()); //ORT
            pst.setInt(6, branch.getVorwahl()); //TEL_LAND
            pst.setLong(7, branch.getTelStartNum()); //TEL_ANFANG_INT
            pst.setString(8, branch.getFaxStartTxt()); //TEL_ANFANG_TXT
            if (branch.getTelEndDefault().equals("")) {
                pst.setObject(9, null);
            } else {
                pst.setInt(9, Integer.parseInt(branch.getTelEndDefault())); //TEL_ENDE_DEFAULT
            }
            pst.setString(10, branch.getFaxStartTxt()); //FAX_ANFANG_TXT
            if (branch.getFaxEndTxt().equals("")) {
                pst.setObject(11, null);
            } else {
                pst.setString(11, branch.getFaxEndTxt()); //FAX_ENDE_TXT
            }
            pst.execute();
        }  
    }  
  
}

Könnte das so passen?

LOL, nein ich meinte nicht die IDE Eclipse sondern die Bibliothek "Eclipselink" der Eclipse Foundation. Eclipselink ist die Referenzimplementierung der JPA (Java Persistence API). Es handelt sich um einen Object-Relational-Mapper (ORM).

Ok, das verstehe ich nicht

 

[mrBrown]

Du solltest den Location-Enum auf zwei Werte begrenzen (DOMESTIC & FOREIGN), sonst kann man zB sowas machen: updateBranch(DOMESTIC_INSERT, ...), was offensichtlicher Unsinn ist

Die Branch-Instanzvariable sollte noch weg, das sollte immer eine rein lokale Variable sein.

Und natürlich f1 - f11 sollten mindesten sinnvoll benannt werden, im Idealfall aber einfach mit einem Parameter vom Typ Branch ersetzt werden.

 

[maGG]

zwei Werte begrenzen (DOMESTIC & FOREIGN), sonst kann man zB sowas machen: updateBranch(DOMESTIC_INSERT, ...), was offensichtlicher Unsinn ist

Aber dann müsste ich 3 EnumMaps machen, was ich ja auch nicht machen soll irgendwie muss ich es ja machen, oder hast du vielleicht ein konkreten Vorschlag, wie ich es besser machen kann?

Habs in lokale Variablen angepasst:

package RepositoryClasses;

import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.EnumMap;
import java.util.List;
import ObjectClasses.Branch;

public class BranchRepository{
    
    public enum Location{
        DOMESTIC, FOREIGN,
        DOMESTIC_UPDATE, FOREIGN_UPDATE,
        DOMESTIC_INSERT,FOREIGN_INSERT
    }
    
    private static final EnumMap<Location, String> queries = new EnumMap<>(Location.class);
    
    static{
        queries.put(Location.DOMESTIC, "SELECT * FROM Filialen_Deutschland");
        queries.put(Location.FOREIGN, "SELECT * FROM Filialen_Ausland");
        queries.put(Location.DOMESTIC_UPDATE, "UPDATE Filialen_Deutschland SET id=?, LABEL=?, STRASZE=?, PLZ_ORT=?, ORT=?,TEL_LAND=?, " +
                "TEL_ANFANG_INT=?, TEL_ANFANG_TXT=?, TEL_ENDE_DEFAULT=?, FAX_ANFANG_TXT=?, FAX_ENDE_TXT=? WHERE id=?");
        queries.put(Location.FOREIGN_UPDATE, "UPDATE Filialen_Ausland SET id=?, LABEL=?, STRASZE=?, PLZ_ORT=?, ORT=?,TEL_LAND=?, " +
                "TEL_ANFANG_INT=?, TEL_ANFANG_TXT=?, TEL_ENDE_DEFAULT=?, FAX_ANFANG_TXT=?, FAX_ENDE_TXT=? WHERE id=?");
        queries.put(Location.DOMESTIC_INSERT, "INSERT INTO Filialen_Deutschland (id,LABEL,STRASZE,PLZ_ORT,ORT,TEL_LAND,TEL_ANFANG_INT," +
                "TEL_ANFANG_TXT,TEL_ENDE_DEFAULT,FAX_ANFANG_TXT,FAX_ENDE_TXT) VALUES(?,?,?,?,?,?,?,?,?,?,?)");
        queries.put(Location.DOMESTIC_INSERT, "INSERT INTO Filialen_Deutschland (id,LABEL,STRASZE,PLZ_ORT,ORT,TEL_LAND,TEL_ANFANG_INT," +
                "TEL_ANFANG_TXT,TEL_ENDE_DEFAULT,FAX_ANFANG_TXT,FAX_ENDE_TXT) VALUES(?,?,?,?,?,?,?,?,?,?,?)");
    }
    
    private final Connection conn;
    
    public BranchRepository(){
        this.conn = null;
    }
    
    public BranchRepository (Connection conn){
        this.conn = conn;
                
    }
    
    public Branch findNames(Location where) throws SQLException{
        String sql = queries.get(where);
        try(PreparedStatement pst = conn.prepareStatement(sql)){
            try(ResultSet rs = pst.executeQuery()){
                Branch b = new Branch();
                while(rs.next()){
                    b.setLabel((rs.getString("Label")));
                }
                return b;             
            }
        }
    }   
    

    public void updateBranch(Location where, int f1, String f2, String f3, String f4, String f5, int f6, long f7, String f8,
            String f9, String f10, String f11) throws SQLException{
        String sql = queries.get(where);
        try (PreparedStatement pst = conn.prepareStatement(sql)) {
            Branch b = new Branch(f1, f2, f3, f4, f5, f6, f7, f8, f9, f10, f11); 
            pst.setInt(1, b.getId()); //id
            pst.setString(2, b.getLabel()); //LABEL
            pst.setString(3, b.getStrasze()); //STRASZE
            pst.setString(4, b.getPlzort()); //PLZ_ORT
            pst.setString(5, b.getOrt()); //ORT
            pst.setInt(6, b.getVorwahl()); //TEL_LAND
            pst.setLong(7, b.getTelStartNum()); //TEL_ANFANG_INT
            pst.setString(8, b.getFaxStartTxt()); //TEL_ANFANG_TXT
            if (b.getTelEndDefault().equals("")) {
                pst.setObject(9, null);
            } else {
                pst.setInt(9, Integer.parseInt(b.getTelEndDefault())); //TEL_ENDE_DEFAULT
            }
            pst.setString(10, b.getFaxStartTxt()); //FAX_ANFANG_TXT
            if (b.getFaxEndTxt().equals("")) {
                pst.setObject(11, null);
            } else {
                pst.setString(11, b.getFaxEndTxt()); //FAX_ENDE_TXT
            }
            pst.setInt(12, b.getId()); //id
            pst.executeUpdate(); 
        }   
    }

    public void insertBranch(Location where, int f1, String f2, String f3, String f4, String f5, int f6, long f7, String f8,
            String f9, String f10, String f11) throws SQLException{
        String sql = queries.get(where);
        try (PreparedStatement pst = conn.prepareStatement(sql)) {
            Branch b = new Branch(f1, f2, f3, f4, f5, f6, f7, f8, f9, f10, f11);
            pst.setInt(1, b.getId()); //id
            pst.setString(2, b.getLabel()); //LABEL
            pst.setString(3, b.getStrasze()); //STRASZE
            pst.setString(4, b.getPlzort()); //PLZ_ORT
            pst.setString(5, b.getOrt()); //ORT
            pst.setInt(6, b.getVorwahl()); //TEL_LAND
            pst.setLong(7, b.getTelStartNum()); //TEL_ANFANG_INT
            pst.setString(8, b.getFaxStartTxt()); //TEL_ANFANG_TXT
            if (b.getTelEndDefault().equals("")) {
                pst.setObject(9, null);
            } else {
                pst.setInt(9, Integer.parseInt(b.getTelEndDefault())); //TEL_ENDE_DEFAULT
            }
            pst.setString(10, b.getFaxStartTxt()); //FAX_ANFANG_TXT
            if (b.getFaxEndTxt().equals("")) {
                pst.setObject(11, null);
            } else {
                pst.setString(11, b.getFaxEndTxt()); //FAX_ENDE_TXT
            }
            pst.execute();
        }   
    }   
    
}

Und natürlich f1 - f11 sollten mindesten sinnvoll benannt werden, im Idealfall aber einfach mit einem Parameter vom Typ Branch ersetzt werden.

Das verstehe ich nicht. Also meinst du die Übergabeparameter?

 

[mihe7]

Nicht

    public void updateBranch(Location where, int f1, String f2, String f3, String f4, String f5, int f6, long f7, String f8,
            String f9, String f10, String f11) throws SQLException{

sondern eher

    public void updateBranch(Location where, Branch branch) throws SQLException{

bzw. ggf. noch besser (das hängt jetzt ein wenig von den Anforderungen ab), wenn Du die Location in den Branch reinnimmst und nur noch

    public void updateBranch(Branch branch) throws SQLException{

hast. Analog dazu natürlich insert etc.

 

[mihe7]

Jetzt habe ich mal was zusammengeschustert. Vielleicht kannst Du damit was anfangen.

In Main musst Du natürlich die DB korrekt einstellen und zum Projekt den passenden JDBC-Treiber hinzufügen.

 

[maGG]

Ah cool, danke für die Mühe! ich verstehe einige Dinge noch nicht, muss mir das noch etwas länger anschauen

 

[mihe7]

Frag einfach, wenn was unklar ist (es sind Teile vorhanden, die gar nicht verwendet werden).

 

[maGG]

Frag einfach, wenn was unklar ist (es sind Teile vorhanden, die gar nicht verwendet werden).

Ok, haha ich habe einige Frage, fangen wir mal mit der Repository Klasse an:

1.
PersonRepository -> getGeneratedId, wieso eine eigene Methode dafür?
stmt.setInt(1, branch.getId()); hätte das nicht auch dafür ausgereicht?

2.
PersonRepository -> fireEvent(RepositoryEvent<Branch> event) -> was macht die Methode?

Ich habe mal eine Testklasse erstellt, diese sieht gerade so aus:

package RepositoryClasses;

import Listener.RepositoryEvent;
import Listener.RepositoryListener;
import ObjectClasses.Branch;
import java.lang.ref.WeakReference;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.EnumMap;
import java.util.Iterator;
import java.util.List;

public class BranchRepository2 {

    public enum Location{ DOMESTIC, FOREIGN }
    
    public enum Update{ DOMESTIC, FOREIGN }
    
    public enum Insert{ DOMESTIC, FOREIGN }
    
    private static final EnumMap<Location, String> LOCATION = new EnumMap<>(Location.class);
    
    private static final EnumMap<Update, String> UPDATE = new EnumMap<>(Update.class);
    
    private static final EnumMap<Insert, String> INSERT = new EnumMap<>(Insert.class);
    
    static{
        LOCATION.put(Location.DOMESTIC, "SELECT * FROM Filialen_Deutschland");
        LOCATION.put(Location.FOREIGN, "SELECT * FROM Filialen_Ausland");
        UPDATE.put(Update.DOMESTIC, "UPDATE Filialen_Deutschland SET id=?, LABEL=?, STRASZE=?, PLZ_ORT=?, ORT=?,TEL_LAND=?, " +
                "TEL_ANFANG_INT=?, TEL_ANFANG_TXT=?, TEL_ENDE_DEFAULT=?, FAX_ANFANG_TXT=?, FAX_ENDE_TXT=? WHERE id=?");
        UPDATE.put(Update.FOREIGN, "UPDATE Filialen_Ausland SET id=?, LABEL=?, STRASZE=?, PLZ_ORT=?, ORT=?,TEL_LAND=?, " +
                "TEL_ANFANG_INT=?, TEL_ANFANG_TXT=?, TEL_ENDE_DEFAULT=?, FAX_ANFANG_TXT=?, FAX_ENDE_TXT=? WHERE id=?");
        INSERT.put(Insert.DOMESTIC, "INSERT INTO Filialen_Deutschland (id,LABEL,STRASZE,PLZ_ORT,ORT,TEL_LAND,TEL_ANFANG_INT," +
                "TEL_ANFANG_TXT,TEL_ENDE_DEFAULT,FAX_ANFANG_TXT,FAX_ENDE_TXT) VALUES(?,?,?,?,?,?,?,?,?,?,?)");
        INSERT.put(Insert.FOREIGN, "INSERT INTO Filialen_Deutschland (id,LABEL,STRASZE,PLZ_ORT,ORT,TEL_LAND,TEL_ANFANG_INT," +
                "TEL_ANFANG_TXT,TEL_ENDE_DEFAULT,FAX_ANFANG_TXT,FAX_ENDE_TXT) VALUES(?,?,?,?,?,?,?,?,?,?,?)");
    }

    private final Connection conn;
    
    public BranchRepository2(){
        this.conn = null;
    }
    
    public BranchRepository2(Connection conn){
        this.conn = conn;
    }

    private final List<WeakReference<RepositoryListener<Branch>>> listeners = new ArrayList<>();
    
    public void addListener(RepositoryListener<Branch> listener) {
        listeners.add(new WeakReference(listener));
    }   
    
    public void removeListener(RepositoryListener<Branch> listener) {
        listeners.remove(new WeakReference(listener));
    }
    
    public List<Branch> findAll(Location where) throws SQLException{
        String sql = LOCATION.get(where);
        try(PreparedStatement pst = conn.prepareStatement(sql)){
            try(ResultSet rs = pst.executeQuery()){
                List<Branch> result = new ArrayList<>();
                while(rs.next()){
                    result.add(readBranch(rs));
                }
                return result;             
            }
        }
    }
    
        public List<String> findLabel(Location where) throws SQLException{
        String sql = LOCATION.get(where);
        try(PreparedStatement pst = conn.prepareStatement(sql)){
            try(ResultSet rs = pst.executeQuery()){
                List<String> result = new ArrayList<>();
                while(rs.next()){
                    result.add(readLabel(rs));
                }
                return result;             
            }
        }
    }
        
    public void updateBranch2(Update where, Branch branch) throws SQLException{
        try(PreparedStatement pst = conn.prepareStatement(UPDATE.get(where))){ 
            pst.setInt(1, branch.getId()); //id
            pst.setString(2, branch.getLabel()); //LABEL
            pst.setString(3, branch.getStrasze()); //STRASZE
            pst.setString(4, branch.getPlzort()); //PLZ_ORT
            pst.setString(5, branch.getOrt()); //ORT
            pst.setInt(6, branch.getVorwahl()); //TEL_LAND
            pst.setLong(7, branch.getTelStartNum()); //TEL_ANFANG_INT
            pst.setString(8, branch.getFaxStartTxt()); //TEL_ANFANG_TXT
            if(branch.getTelEndDefault().equals("")){
                pst.setObject(9, null);
            }else{
                pst.setInt(9, Integer.parseInt(branch.getTelEndDefault())); //TEL_ENDE_DEFAULT
            }
            pst.setString(10, branch.getFaxStartTxt()); //FAX_ANFANG_TXT
            if(branch.getFaxEndTxt().equals("")){
                pst.setObject(11, null);
            }else{
                pst.setString(11, branch.getFaxEndTxt()); //FAX_ENDE_TXT
            }
            pst.setInt(12, branch.getId()); //id
            pst.executeUpdate();
            fireUpdated(branch);
        }   
    }         
    
    public void insertBranch2(Insert where, Branch branch) throws SQLException{
        try (PreparedStatement pst = conn.prepareStatement(INSERT.get(where))) {
            pst.setInt(1, branch.getId()); //id
            pst.setString(2, branch.getLabel()); //LABEL
            pst.setString(3, branch.getStrasze()); //STRASZE
            pst.setString(4, branch.getPlzort()); //PLZ_ORT
            pst.setString(5, branch.getOrt()); //ORT
            pst.setInt(6, branch.getVorwahl()); //TEL_LAND
            pst.setLong(7, branch.getTelStartNum()); //TEL_ANFANG_INT
            pst.setString(8, branch.getFaxStartTxt()); //TEL_ANFANG_TXT
            if (branch.getTelEndDefault().equals("")) {
                pst.setObject(9, null);
            } else {
                pst.setInt(9, Integer.parseInt(branch.getTelEndDefault())); //TEL_ENDE_DEFAULT
            }
            pst.setString(10, branch.getFaxStartTxt()); //FAX_ANFANG_TXT
            if (branch.getFaxEndTxt().equals("")) {
                pst.setObject(11, null);
            } else {
                pst.setString(11, branch.getFaxEndTxt()); //FAX_ENDE_TXT
            }
            pst.execute();
            fireAdded(branch);
        }   
    }     
    
    private void fireAdded(Branch branch) {
        fireEvent(new RepositoryEvent<>(RepositoryEvent.Type.ADDED, branch));
    }

    private void fireUpdated(Branch branch) {
        fireEvent(new RepositoryEvent<>(RepositoryEvent.Type.UPDATED, branch));
    }
    
    private void fireEvent(RepositoryEvent<Branch> event) {
        Iterator<WeakReference<RepositoryListener<Branch>>> it = listeners.iterator();
        while (it.hasNext()) {
            WeakReference<RepositoryListener<Branch>> ref = it.next();
            RepositoryListener<Branch> listener = ref.get();
            if (listener != null) {
                listener.eventOccured(event);           
            } else {
                it.remove();
            }
        }
    }       
        
    private Branch readBranch(ResultSet rs) throws SQLException {
        return new Branch(rs.getInt(1), rs.getString(2), rs.getString(3), rs.getString(4), rs.getString(5), rs.getInt(6), rs.getLong(7), rs.getString(8), rs.getString(9), rs.getString(10), rs.getString(11));
    }
        
    private String readLabel(ResultSet rs) throws SQLException{
        return rs.getString(2);
    }
    
}

Könnte das so evtl. passen?

Ich weiß noch auch noch nicht wie ich mein Renderer anwende auf meine "jComboBoxFilialenDeutschland" anwende und ob ich alle Parameter beim Renderer brauche, aber bleiben wir vielleicht erst mal bei der Repository Klasse :O

 

[mihe7]

PersonRepository -> getGeneratedId, wieso eine eigene Methode dafür?
stmt.setInt(1, branch.getId()); hätte das nicht auch dafür ausgereicht?

Das kommt darauf an, wie Du die ID erzeugst. In meinem Beispiel gehe ich von einem auto_increment Feld in MySQL aus. Bei einem INSERT INTO person (name, vorname) VALUES (...) erzeugt die Datenbank automatisch die ID.

Das Person-Objekt hat zum Zeitpunkt des Aufrufs von add() noch keine ID. Die Methode getGeneratedId() lässt sich die von der DB beim INSERT vergebene ID zurückgeben. Anschließend wird diese verwendet ,um die ID des Person-Objekts zu setzen. D. h. nach dem Aufruf von add() hat das Person-Objekt eine ID und zwar diejenige, die von der DB vergeben wurde.

Wenn der Benutzer die ID selbst eingibt oder die Anwendung die ID irgendwie vergibt (z. B. in Form eines Zählers oder einer UUID), dann kannst Du natürlich der Person die ID auch vor dem add mitgeben und dann z. B. stmt.setInt(1, person.getId()) verwenden.

PersonRepository -> fireEvent(RepositoryEvent<Branch> event) -> was macht die Methode?

Die Methode benachrichtigt registrierte Listener über ein eingetretenes Ereignis. Sie wird intern von fireAdded bzw. fireUpdated (und wenn es programmiert wäre, dann auch von fireRemoved etc.) verwendet. Allerdings existiert in dem Beispiel (wenn ich mich recht entsinne) kein Listener, der darauf reagieren würde.

Wozu kann man das brauchen? Das Person-Repository ist die zentrale Anlaufstelle, wenn es um Personen geht. Es bekommt jede persistente Änderung mit. Das Observer-Pattern (nichts anderes ist das) kann verwendet werden, um völlig voneinander unabhängige Programmteile miteinander kommunizieren zu lassen.

Aktuell läuft das ausschließlich über das UI-Model (PersonsModel), weil dieses sowohl TableModel, ListModel und ComboBoxModel implementiert und somit für alles verwendet wird und ich die Sache nicht weiter verkomplizieren wollte.

Aber stell Dir mal vor, Du baust einen Editor für eine Person. Der bekommt das Person-Objekt und das Repository mit. Der Benutzer bearbeitet die Daten und klickt auf "Speichern". Der Editor ruft dann die udpate-Methode des Repositories auf. Das Repository benachrichtigt nun alle Observer über die Änderung. So kann z. B. ein TableModel (das sich als Listener beim Repository registriert hat) feststellen, dass sich eine Zeile geändert hat und die Daten sofort aktualisieren.

Oder geht es Dir darum, wie die programmiert ist? Hier verwende ich WeakReferences zur Vermeidung von Memory Leaks. Hintergrund ist, dass der Speicher eines Objekts erst freigegeben werden kann, wenn es keine Referenz mehr auf dieses Objekt gibt.

Stell Dir mal folgende Situation vor: Du erstellst ein Fenster, das die Personen anzeigt. Dort wird eine JTable verwendet, ein entsprechendes TableModel, das die Daten cached usw. Jetzt gehen wir mal davon aus, dass sich dieses Fenster als Listener beim Repository registriert. Damit gibt es eine Referenz vom Repository zum Fenster. Du musst also sicherstellen, dass die Registrierung unter allen Umständen aufgehoben wird, wenn das Fenster nicht mehr benötigt wird. Ansonsten bleibt die Referenz auf das Fenster bestehen und der Speicher kann nicht freigegeben werden.

Aus diesem Grund verwende ich eine WeakReference. Ist ein Objekt nur noch über schwache Referenzen erreichbar, werden diese automatisch entfernt. In dem Fall liefert WeakReference#get() null zurück. Die fireEvent-Methode entfernt die betreffenden Listener automatisch aus der Liste.

Das Gegenbeispiel findest Du in PersonsModel. Dort werden normale Referenzen verwendet. Da sich alle Fenster das gleiche Model teilen, hast Du hier einen Memory-Leak. Der Speicher wird erst freigegeben, wenn alle Referenzen auf das PersonsModel aufgehoben wurden. Dies ist aber erst der Fall, wenn das letzte Fenster geschlossen wurde. Bis dahin werden alle jemals geöffneten Fenster (bzw. die JTables und JComboBoxes) weiterhin als Listener geführt (und natürlich auch benachrichtigt); selbst wenn sie längst geschlossen wurden. Um das zu beheben müsste man einen WindowListener einbauen, der für z.B. JTable setModel(null) aufruft (dann hebt die JTable die Registrierung auf).

Könnte das so evtl. passen?

Du brauchst nur die Location und die verwendest Du mehrfach, z. B.

    private static final EnumMap<Location, String> SELECT = new EnumMap<>(Location.class);
    private static final EnumMap<Location, String> UPDATE = new EnumMap<>(Location.class);    
    private static final EnumMap<Location, String> INSERT = new EnumMap<>(Location.class);

Es geht ja nur darum, die passenden SQL-Strings zu finden. Du kannst Dir überlegen, ob Du die Location nicht in die Branch aufnimmst. Dann kannst Du in den Methoden insert und update auf den Location-Parameter verzichten. Apropos Update: dort brauchst Du die ID nicht zweimal (UPDATE ... SET id = ? ... WHERE id=?). Andererseits: wenn bei Dir die stmt.setXXXX-Methoden dann identisch sind, kannst Du die in eine Methode auslagern, dann hast Du den Code nicht doppelt.

 

[maGG]

Das Person-Objekt hat zum Zeitpunkt des Aufrufs von add() noch keine ID. Die Methode getGeneratedId() lässt sich die von der DB beim INSERT vergebene ID zurückgeben. Anschließend wird diese verwendet ,um die ID des Person-Objekts zu setzen. D. h. nach dem Aufruf von add() hat das Person-Objekt eine ID und zwar diejenige, die von der DB vergeben wurde.

Das Problem habe ich glaube ich nicht, weil man im Editierfenster auch ein Textfield für die id hat und dieses muss gesetzet werden, zudem würde man das Problem einer fehlenden id nicht damit beheben, dass man setId() in die erste Zeile schreibt, wie bei mir?

Du brauchst nur die Location und die verwendest Du mehrfach, z. B.

Ah cool, auf die Idee kam ich nicht

Apropos Update: dort brauchst Du die ID nicht zweimal (UPDATE ... SET id = ? ... WHERE id=?)

Hmm brauch ich die in meinem Fall nicht dennoch? Weil ich ein Textfeld habe, in der der Nutzer die id setzen kann.

Aktuell läuft das ausschließlich über das UI-Model (PersonsModel), weil dieses sowohl TableModel, ListModel und ComboBoxModel implementiert und somit für alles verwendet wird und ich die Sache nicht weiter verkomplizieren wollte.

Die abstrakte Klasse Person Model verstehe ich noch nicht; hätte man das nicht noch in PersonRepository machen können? Oder geht es darum, dass man so immer ein Refresh bei der Ansicht der Tabelle bekommt? Das hatte ich bisher so gelöst, dass ich in dem Button die entsprechende Methode erst aufgerufen habe und die Methode für das Anzeige der Datenbank in der Tabelle einfach wiederhole.

Registrierung unter allen Umständen aufgehoben wird, wenn das Fenster nicht mehr benötigt wird. Ansonsten bleibt die Referenz auf das Fenster bestehen und der Speicher kann nicht freigegeben werden.

Das verstehe ich nicht :O ist das noch was anderes, warum ich die try-ressources Methode verwende, und nich mehr pst.close() und rs.close()?

WeakReference. Ist ein Objekt nur noch über schwache Referenzen

Ok, das heißt ich brauche den unbedingt für eine Event Funktion oder sollte es zu mindest benutzen?

selbst wenn sie längst geschlossen wurden. Um das zu beheben müsste man einen WindowListener einbauen, der für z.B. JTable setModel(null) aufruft (dann hebt die JTable die Registrierung auf).

Puh, das verstehe ich auch noch nicht. Also ich muss alle Referenzen löschen? Ich höre davon zum ersten Mal; gilt das überall oder nur bei jTable?

Nochmal vielen Dank für die Hilfe!

 

[mihe7]

zudem würde man das Problem einer fehlenden id nicht damit beheben, dass man setId() in die erste Zeile schreibt, wie bei mir?

Wenn Du die ID setzen kannst: ja. Meist hat man das Problem, dass man die ID nicht kennt also irgendwie ermitteln muss. In meinem Beispiel wird die Ermittlung der ID der DB überlassen.

Hmm brauch ich die in meinem Fall nicht dennoch? Weil ich ein Textfeld habe, in der der Nutzer die id setzen kann.

Dann natürlich schon. Es ist allerdings ungewöhnlich, dass a) der Benutzer die ID bestimmt und b) der Primary Key im Nachhinein geändert wird (das führt zu Problemen, wenn es Beziehungen gibt).

Auf den Rest gehe ich mal durch Beantwortung der folgenden Frage ein.

Puh, das verstehe ich auch noch nicht. Also ich muss alle Referenzen löschen? Ich höre davon zum ersten Mal; gilt das überall oder nur bei jTable?

Der Grundsatz gilt immer: so lange ein Objekt über den Objektbaum erreichbar ist, kann der Speicher nicht freigegeben werden. Allerdings muss man meist nicht alle Referenzen löschen, die Wurzel eines Teilbaums reicht.

Dein Objektbaum wird ja durch die Ausführung Deines Programms aufgebaut. Machen wir mal ein einfaches Beispiel:

public static void main(String[] args) {
    List<Element> liste = new ArrayList<>();
}

Hier passieren mehrere Dinge.
1. Die JVM ruft die main-Methode auf. Damit wird im Stack des main-Threads ein Stackframe für diesen Methodenaufruf angelegt
2. Es wird ein ArrayList-Objekt im Heap erzeugt.
3. Die Referenz auf dieses Objekt wird in einer lokalen Variable gespeichert, die im Stackframe der Methode abgelegt wird.
4. Beim Beenden den Methode wird der Stackframe (und damit die lokale Variable/Referenz auf das ArrayList-Objekt) entfernt.

Damit ist klar: ab 3. existiert eine Referenz auf das ArrayList-Objekt. Und so lange diese existiert, kann der Speicher nicht freigegeben werden. Nach Beendigung der Methode ist die Referenz weg (weil lokale Variable), womit der für die Liste benötigte Speicher freigegeben werden kann.

Es spielt in dem Fall auch keine Rolle, ob in der Liste noch Objekte stecken, die die Liste referenzieren: von der Wurzel des Objektbaums aus ist die Liste nicht mehr erreichbar, folglich kann die Liste freigegeben werden.

Beispiel:

public static void main(String[] args) {
    List<Element> liste = new ArrayList<>();
    liste.add(new Element(liste));
}

Würden wir das ganze aber umschreiben:

public void add(Element e) {
    List<Element> liste = new ArrayList<>();
    e.setList(liste);
    liste.add(e);
}

Dann haben wir das Problem, dass die Liste nicht freigegeben werden kann, so lange das übergebene Element existiert.

 

[maGG]

Achso, und mit liste.add(new Element(liste)); würde ich eine neue Referenz haben und dadurch würde ich nicht diesselbe Referenz benutzen, die ja erst nach Beendigung der Methode wieder verfügbar wäre? Und wenn ich die add-Methode in einer neuen Methode aufrufe ginge es ja auch, hast du deswegen extra noch eine PersonModel Klasse eingeführt?
Oder ist der Grund für PersonModel und PersonView eher die Visualisierung des Listeners geschuldet?

Wo verwendest du denn den Renderer bzw. wie benutzen ich den? Wenn ich jetzt List<Branch>findAll(Location where) oder List<String>findLabel(Location where) aufrufe und damit die ComboBox befüllen möchte. Wie würde ich den Renderer bei findAll benutzen?
Wenn ich dich richtig verstande haben war die Idee ja folgende: Anstatt mir nur Label als String aus der Db zu holen, hole ich mir doch lieber alle Werte und mit einem Renderer dann nur das Label hinzu. So könnte ich evtl. in der ActionPerformed Methode des Buttons nicht noch zusätzlich eine set Methode für die Übergabe Parameter für die Writer Methode später, oder habe ich das falsch verstanden?

 

[mihe7]

Achso, und mit liste.add(new Element(liste)); würde ich eine neue Referenz haben und dadurch würde ich nicht diesselbe Referenz benutzen, die ja erst nach Beendigung der Methode wieder verfügbar wäre?

Ja, mit liste.add(new Element(liste)); würdest Du ein neues Element-Objekt erzeugen, das intern die lokale Liste referenziert (davon gehen wir zumindest aus) und die Referenz auf das Element würdest Du in der Liste speichern (Element und Liste referenzieren sich also gegenseitig). Nach Beendigung der Methode kann in dem Fall liste jedoch freigegeben werden, da die Referenz auf die liste nur in der Liste existiert (das Element, das die Liste referenziert, wird ja nur über die Liste referenziert).

In der zweiten Methode kommt das Element "von außerhalb" (via Parameter) und es wird eine Referenz auf die lokal erstellte Liste gesetzt. Wenn also das Element außerhalb weiterhin referenziert wird, dann kann die lokal erstellte Liste nicht freigegeben werden.

Deutlich wird es z. B. so:

class TestObjekt {
    private Element element = new Element();

    public void add() {
        List<Element> liste = new ArrayList<>();
        liste.add(element);
        element.setList(liste);
    }

    public void show() {
        List<Element> imElementReferenzierteListe = element.getList();
        // ...
    }

    public void clear() {
        element = null;
    }
}

Nehmen wir jetzt folgenden Code an:

TestObjekt a = new TestObjekt();
a.add();

Dann hast Du folgenden Objektbaum:

-->a ---> element <--> liste

So lange also eine Referenz auf element existiert, kann die Liste nicht freigegeben werden, denn Du könntest ja irgendwann aufrufen:

a.show();

Und dort wird die Liste wieder benötigt.

Würdest Du dagegen a.clear(); aufrufen, würde die Referenz auf das Element entfernt:

-->a       element <--> liste

Damit ist im Objektbaum nur noch a erreichbar, element und liste jedoch nicht mehr. Und erst jetzt kann auch die liste freigegeben werden.

hast du deswegen extra noch eine PersonModel Klasse eingeführt?
Oder ist der Grund für PersonModel und PersonView eher die Visualisierung des Listeners geschuldet?

Das PersonModel implementiert einfach TableModel und ComboBoxModel. Damit werden die JTable/JComboBox nicht mehr aktiv gefüllt. Das PersonModel dient einfach als Adapter, um das Repository an eine JTable/JComboBox anbinden zu können, ohne irgendwelche fill-Methoden aufrufen zu müssen.

Wo verwendest du denn den Renderer bzw. wie benutzen ich den? Wenn ich jetzt List<Branch>findAll(Location where) oder List<String>findLabel(Location where) aufrufe und damit die ComboBox befüllen möchte. Wie würde ich den Renderer bei findAll benutzen?

Man gibt der JComboBox einfach an, mit welchem Renderer sie die Elemente darstellen solll (s. JComboBox#setRenderer). Sprich:

BranchRenderer renderer = new BranchRenderer();
JComboBox filialenInland = new JComboBox(...);
JComboBox filialenAusland = new JComboBox(...);
filialenInland.setRenderer(renderer);
filialenAusland.setRenderer(renderer);

Wenn ich dich richtig verstande haben war die Idee ja folgende: Anstatt mir nur Label als String aus der Db zu holen, hole ich mir doch lieber alle Werte und mit einem Renderer dann nur das Label hinzu. So könnte ich evtl. in der ActionPerformed Methode des Buttons nicht noch zusätzlich eine set Methode für die Übergabe Parameter für die Writer Methode später, oder habe ich das falsch verstanden?

Genau, die Idee ist, mit den Objekten des Problembereichs zu arbeiten (wofür haben wir OO), statt auf ggf. primitive Einzelteile zurückzugreifen. Du lädst also nicht Labels aus der DB, sondern Filialen. Und die Filialen kannst Du dann sowohl für die JComboBox verwenden (der Renderer sorgt dafür, dass die Anzeige passt), als auch z. B. für die Tabelle.

Genauso wie im Beispiel

Ein Vorteil ist: der Benutzer wählt jetzt nicht nur ein Label aus, sondern eben eine Filiale. Damit hast Du unmittelbar nach der Auswahl ein Branch-Objekt und damit sämtliche Eigenschaften/Methoden des selben zur Verfügung.

 

[maGG]

Ok, danke! Ich habe die Klasse PersonObjekt jetzt mal für Branch angelegt, jedoch tue ich mich grad bei der Logik der zu übergehenden Parameter in den Methoden add und update schwer:

Hier verstehe ich noch nicht ganz, warum wir List<Branch> data verwenden und wie ich ein Objekt der Klasse Branch nun diesem hinzufügen können.

/*
 * To change this license header, choose License Headers in Project Properties.
 * To change this template file, choose Tools | Templates
 * and open the template in the editor.
 */
package ModelClasses;

import ObjectClasses.Branch;
import RepositoryClasses.BranchRepository;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.List;
import javax.swing.ComboBoxModel;
import javax.swing.ListModel;
import javax.swing.event.EventListenerList;
import javax.swing.event.ListDataEvent;
import javax.swing.event.ListDataListener;
import javax.swing.table.AbstractTableModel;
import javax.swing.table.TableModel;

/**
 *
 * @author david
 */
public class BranchModel extends AbstractTableModel implements TableModel, ListModel<Branch>, ComboBoxModel<Branch>{
 
    private EventListenerList listeners = new EventListenerList();

    private static final String[] COLUMNS =
    {"Label", "Strasze", "PLZ Ort", "Ort", "Vorwahl", "Tel.Htm.Anfang", "Tel.Txt.Anfang", "Tel.Ende", "Fax.Txt.Anfang", "Fax.Txt.Ende"};

    private BranchRepository branches;
    private BranchRepository.Location location;
    private List<Branch> data;
    private Object selected;
    
    public BranchModel(BranchRepository branches, BranchRepository.Location location) {
        this.branches = branches;
        this.location = location;
        try {
            data = branches.findAll(location);
        } catch (SQLException e) {
            data = new ArrayList<>();
            e.printStackTrace();
        }
    }   
    
    public void add(BranchRepository.Location where, Branch branch) {
        try {
            branches.insertBranch(where, branch);
            int ix = getSize();
            data.insertBranch(where, branch); //hier wird ein Fehler angezeigt: cannot find symbol
            fireTableRowsInserted(ix, ix);
            fireIntervalAdded(ix, ix);
        } catch (SQLException e) {
            e.printStackTrace();
        }
    } 
    
    public void update(BranchRepository.Location where, Branch branch) {
        try {
            branches.updateBranch(where, branch);
            int ix = data.indexOf(branch);
            fireContentsChanged(ix, ix);
            fireTableRowsUpdated(ix, ix);
        } catch (SQLException ex) {
            ex.printStackTrace();
        }
    }   

    protected void fireIntervalAdded(int index0, int index1){
        ListDataEvent event = new ListDataEvent(this, ListDataEvent.INTERVAL_ADDED, index0, index1);
        for (ListDataListener l : listeners.getListeners(ListDataListener.class)) {
            l.intervalAdded(event);
        }
    }

    protected void fireIntervalRemoved(int index0, int index1){
        ListDataEvent event = new ListDataEvent(this, ListDataEvent.INTERVAL_REMOVED, index0, index1);
        for (ListDataListener l : listeners.getListeners(ListDataListener.class)) {
            l.intervalRemoved(event);
        }
    }

    protected void fireContentsChanged(int index0, int index1){
        ListDataEvent event = new ListDataEvent(this, ListDataEvent.CONTENTS_CHANGED, index0, index1);
        for (ListDataListener l : listeners.getListeners(ListDataListener.class)) {
            l.contentsChanged(event);
        }
    }       
    
    //implemented abstract methodes
    
    @Override
    public int getSize(){
            return data.size();
    }

    @Override
    public Branch getElementAt(int index){
            return data.get(index);
    }

    @Override
    public void addListDataListener(ListDataListener l){
            listeners.add(ListDataListener.class, l);
    }

    @Override
    public void removeListDataListener(ListDataListener l){
            listeners.remove(ListDataListener.class, l);
        }
    
    @Override
    public int getRowCount(){
            return getSize();
    }

    @Override
    public int getColumnCount(){
            return COLUMNS.length;
        }
    
        @Override
        public String getColumnName(int column){
            return COLUMNS[column];
        }

    @Override
    public Object getValueAt(int rowIndex, int columnIndex){
        Branch branch = getElementAt(rowIndex);
            switch (columnIndex){
                case 0: return branch.getId();
                case 1: return branch.getVorwahl();
                case 2: return branch.getTelStartNum();
                case 3: return branch.getLabel();
                case 4: return branch.getStrasze();
                case 5: return branch.getPlzort();       
                case 6: return branch.getOrt();       
                case 7: return branch.getTelStartTxt();
                case 8: return branch.getTelEndDefault();       
                case 9: return branch.getFaxStartTxt() ;       
                case 10: return branch.getFaxEndTxt() ;       
                default: return null;
            }
    }
        
    @Override
    public void setSelectedItem(Object anItem){
            selected = anItem;
    }

    @Override
    public Object getSelectedItem(){
            return selected;
    }
        
}

Bei PersonView hast du mit einer setCurrent Methode direkt in dem Element gearbeitet, bei mir in Netbeans funktioniert das alles ein bisschen anders. Daher weiß ich leider immer noch nicht wie ich z.B: bei jComboBoxFilialenDeutschland den Renderer anwende.

Meine ComboBox sieht gerade so aus:

jComboBoxFilialenDeutschland = new javax.swing.JComboBox<>();

jComboBoxFilialenDeutschland.setModel(new javax.swing.DefaultComboBoxModel<>(new String[] { "Bitte auswählen" }));

jComboBoxFilialenDeutschland.addActionListener(new java.awt.event.ActionListener() {
    public void actionPerformed(java.awt.event.ActionEvent evt) {
        jComboBoxFilialenDeutschlandActionPerformed(evt);
    }
});

// Code adding the component to the parent container - not shown here

Könnte ich da vielleichet auch den Teil von PersonView reinpacken und Methoden wie setCurrent in meiner Hauptklasse lassen?

 

[mihe7]

Arbeitest Du mit der UI-Builder von NetBeans? Es ist schon ewig her, dass ich den benutzt habe aber ich weiß, dass man das dem UI-Editor beibrignen kann/konnte. Du kannst in den Properties Code eingeben. Und es gibt auch Pre-/Post-Init-Code. Genaueres kann ich Dir gerade nicht sagen, dazu müsste ich den Editor erst anschmeißen. Falls Du da Hilfe brauchst, sag Bescheid, dann werfe ich den Editor an.

Hier verstehe ich noch nicht ganz, warum wir List<Branch> data verwenden

Als Cache, damit wir nicht ständig auf der DB rumfuhrwerken.

Vereinfachen wir das ganze mal, indem wir uns auf die JComboBox konzentrieren (=> ComboBoxModel) und Scrolling ignorieren.

Du musst Dir das so vorstellen: die JComboBox will die Liste mit den Einträgen anzeigen. Dazu fragt sie beim ComboBoxModel nach, wie viele Einträge es gibt, indem es die getSize()-Methode des ComboBoxModels aufruft. Anschließend ruft sie die betreffenden Elemente vom ComboBoxModel ab (getElementAt(index)), gibt das jeweilige Element an den gesetzten Renderer weiter, der dann eine UI-Komponente zurückliefert. Die JComboBox ruft dann die paint-Methode der UI-Komponente auf, um das Teil zu zeichnen.

Der wichtige Punkt dabei ist: der JComboBox ist es völlig egal, wo die Daten herkommen und welche Daten es sind. Einzige Voraussetzung ist, dass das Model-Objekt der JComboBox das ComboBoxModel-Interface implementiert. Den Punkt muss man verstanden haben, wenn man mit Swing (und anderen UI-Frameworks) Freude haben will (Google mal nach MVC Pattern).

Aus Sicht der JComboBox könnte also auch das Repository das ComboBoxModel direkt implementieren (getSize() könnte z. B. das Ergebnis von "SELECT count(*) ..." zurückgeben) . Das Repository würde dabei aber ständig auf die DB zugreifen, was die Sache sehr langsam machen würde. Daher verwenden wir eine List als Cache und eine separate Klasse weil die Bereitstellung von Daten im "JComboBox-Format" nichts mit der DB zu tun und somit nichts im Repository verloren hat.

data.insertBranch(where, branch); //hier wird ein Fehler angezeigt: cannot find symbol

Ja, die insertBranch-Methode des Repositories gibt es in der List nicht Änder einfach data zu repository und füg eine zweite Zeile data.add(branch); hinzu. Die insertBranch-Methode sorgt dafür, dass das Repository (also die DB) aktualisiert wird, die data.add()-Methode, dass der Cache aktualisiert wird.

 

[maGG]

Vielen Dank für die Hilfe! Ich habe gestern länger darüber nachgedacht und glaube ich bin jetzt grad an nem Punkt, wo es mehr um Logik geht bzw. wie ich es aufbaue. Dein sehr schönes Beispiel, wie ich es machen könnte lässt sich nicht ganz auf meinen Anwendungsfall anwenden, zu mindest nicht 1:1.

Ich habe insgesamt 5 Tabellen in der Datenbank:
Filialen_Deutschland(11 Parameter), Filialen_Ausland(11 Parameter), Firmierungen(2 Parameter), Titel(3 Parameter), Funktionen (2 Parameter)

Dementsprechend habe ich auch 5 Tabellen bzw. 5 Seiten bei einem TabbedPane.

Auf den Tabellen habe ich jeweils zwei Methoden:

private void jTableTitelKeyReleased(java.awt.event.KeyEvent evt) {
    if (evt.getKeyCode() == KeyEvent.VK_UP || evt.getKeyCode() == KeyEvent.VK_DOWN) {
    ...
    }
}

private void jTableTitelMouseClicked(java.awt.event.MouseEvent evt) {
    ...
}

Dann habe ich noch 5 ComboBox(en), für die jeweiligen Tabellen, wo nur das Label angezeigt wird. Von der Logik her müsste ich doch dann in meiner setCurrent Methode die Parameter mit Werten belegen, die ich später in der Writer Methode brauche und die in der Vorschau Methode angezeigt werden.

Zum Beispiel ich setzen bei der ComboBox GermanBranches jetzt das Label Frankfurt am Main, dann müsste die setCurrent Methode alle Parameter setzen und danach wird die Preview Methode aufgerufen und das Vorschaufenster aktualisiert.

Wenn ich jetzt in einer der Tabellen etwas editieren möchte, dann setzt er bei einem Action Event die entsprechenden Textfelder mit Werten. Bei Speicher und Update holt er sich die Werte. Das heißt ich hab doch quasi eine Trennng zwischen ComboBox und Table, oder sehe ich das falsch?

Ich kriegs gedanklich nicht hin, deine wohlüberlegte Struktur darauf anzuwenden. Für die Repository Klassen habe ich das Ganze noch hinbekommen, aber wie ich die Model und dann die View Klassen darauf anwenden kann sprengt gerade etwas meine Vorstellungskraft. Leider kann ich nicht mit Parameter wie JComboBoxFilialenDeutschland nicht außerhalb der initComponent Methode arbeiten, sobald ich sie außerhalb benutze habe ich kein Zugriff darauf, was hieße ich müsste für jede Action Event Methode quasie densleben Code nochmal reinschreiben. Ein Teil erspare ich mir durch eine Repository Klasse, aber so schön wie das bei dir ist krieg ich das irgendwie nicht hin, oder mir fällt zu mindest keine Lösung ein.

Ich grüble mal weiter, vielleicht fällt mir ja doch noch was ein

 

[mihe7]

Kannst Du mir mal Deinen aktuellen Code schicken, damit ich das nachvollziehen kann, was Du meinst?

Ein Punkt ist völlig klar: 1:1 kannst Du das nicht übernehmen, da der Code bei mir Quick & Dirty davon ausgeht, dass es ein einziges PersonsModel gibt. Das kannst Du so nicht machen, weil Du mehrere JComboBoxen hast, und jede JComboBox "ihr eigenes" setCurrent/getCurrent braucht.

Zwei Dinge verstehe ich nicht:

Von der Logik her müsste ich doch dann in meiner setCurrent Methode die Parameter mit Werten belegen, die ich später in der Writer Methode brauche und die in der Vorschau Methode angezeigt werden.

und

bei der ComboBox GermanBranches jetzt das Label Frankfurt am Main, dann müsste die setCurrent Methode alle Parameter setzen und danach wird die Preview Methode aufgerufen und das Vorschaufenster aktualisiert.

Wenn Du mit Objekten arbeitest, wird nicht ein "Label" Frankfurst am Main gesetzt sondern die komplette Filiale. Aber, lass mal den Code sehen.

 

[maGG]

Kannst Du mir mal Deinen aktuellen Code schicken, damit ich das nachvollziehen kann, was Du meinst?

Klar, ich weiß jedoch nicht ob du dich zurecht finden wirst; ich habe vieles wieder auskommentiert und Code noch drin, der am Ende raus soll ... es ist ein Chaos im Moment

Ich habe gestern noch rumprobiert; ich habe in meine BranchView Klasse die Hauptklasse importiert (k.A. ob man das machen sollte) und dann die einzelnen Textfelder im GUI Builder auf public gesetzt. Jetzt habe ich zwar Zugriff darauf, jedoch hab ich das static Problem; "Non-static variable cannot be referenced from a static context" Könnte das an der Run Methode liegen? (public static void main (String args[]))

 

[maGG]

update: habe das static Problem gelöst; habe folgendes gemacht:
import Interface.Main
private Main main;
...
main.jTextfield.setText(...);

 

[maGG]

Meine Klasse sieht jetzt so aus:

package ModelClasses;

import ObjectClasses.Branch;
import RepositoryClasses.BranchRepository;
import UserInterface.Main;

public class BranchViewDomestic {
    
    private BranchModel branches;
    private Branch current;
    private Main main;
    
    public BranchViewDomestic(BranchModel branches) {
        this.branches = branches;
    }   

    private void setCurrent(Branch branch) {
        this.current = branch;
        boolean valid = current != null;
        if (valid) {
            main.jTextFieldF1.setText("" + current.getId());
            main.jTextFieldF2.setText(current.getLabel());
            main.jTextFieldF3.setText(current.getStrasze());
            main.jTextFieldF4.setText(current.getPlzort());       
            main.jTextFieldF5.setText(current.getOrt());
            main.jTextFieldF6.setText("" + current.getVorwahl());
            main.jTextFieldF7.setText("" + current.getTelStartNum());
            main.jTextFieldF8.setText(current.getTelStartTxt());
            main.jTextFieldF9.setText("" + current.getTelEndDefault());
            main.jTextFieldF10.setText(current.getFaxStartTxt());
            main.jTextFieldF11.setText(current.getFaxEndTxt());
        } else {
            main.jTextFieldF1.setText("");
            main.jTextFieldF2.setText("");
            main.jTextFieldF3.setText("");
            main.jTextFieldF4.setText("");       
            main.jTextFieldF5.setText("");
            main.jTextFieldF6.setText("");
            main.jTextFieldF7.setText("");
            main.jTextFieldF8.setText("");
            main.jTextFieldF9.setText("");
            main.jTextFieldF10.setText("");
            main.jTextFieldF11.setText("");
        }
        main.jTextFieldF1.setEnabled(valid);
        main.jTextFieldF2.setEnabled(valid);
        main.jTextFieldF3.setEnabled(valid);
        main.jTextFieldF4.setEnabled(valid);     
        main.jTextFieldF5.setEnabled(valid);
        main.jTextFieldF6.setEnabled(valid);
        main.jTextFieldF7.setEnabled(valid);
        main.jTextFieldF8.setEnabled(valid);
        main.jTextFieldF9.setEnabled(valid);
        main.jTextFieldF10.setEnabled(valid);
        main.jTextFieldF11.setEnabled(valid);
        main.jButtonUpdateA1.setEnabled(valid);
        main.jButtonSaveA1.setEnabled(valid);
        main.jButtonDeleteA1.setEnabled(valid);   
    }

    private void save() {
        if (this.current == null) {
            return;
        }
        current.setId(Integer.parseInt(main.jTextFieldF1.getText()));
        current.setLabel(main.jTextFieldF2.getText());
        current.setStrasze(main.jTextFieldF3.getText());
        current.setPlzort(main.jTextFieldF4.getText());
        current.setOrt(main.jTextFieldF5.getText());
        current.setVorwahl(Integer.parseInt(main.jTextFieldF6.getText()));
        current.setTelStartNum(Long.parseLong(main.jTextFieldF7.getText()));
        current.setTelStartTxt(main.jTextFieldF8.getText());
        current.setTelEndDefault(Integer.parseInt(main.jTextFieldF9.getText()));
        current.setFaxEndTxt(main.jTextFieldF10.getText());
        current.setFaxEndTxt(main.jTextFieldF11.getText());
        if (current.getId() == 0L) {
            branches.add(BranchRepository.INSERT<Location.DOMESTIC>, current); //<- falsch
            //BranchRepository.DOMESTIC geht aber ebenfalls nicht, da ich sonst keine Unterscheidung habe, ob es sich um LOCATION, UPDATE, oder INSERT als EnumMap handelt
        } else {
            branches.update(BranchRepository.INSERT<Location.DOMESTIC>, current); // falsch
        }
    }   
}

 

[mrBrown]

Sind die Kommentare im Code die Fragen?

Falls ja:

Die Aufrufe sollten branches.add(Location.DOMESTIC, current);[icode] bzw [icode]branches.update(Location.DOMESTIC, current); sein. Ob es SELECT, UPDATE, oder INSERT ist, ist durch den Methodenaufruf klar - die Location soll nicht zur Unterscheidung dessen dienen, sondern eben nur als Location.





Und ein paar kleine Tipps am Rande:
Zieh die Location mit in den Branch rein. Auch wenn das nicht als Wert in der Tabelle steht, sondern nur durch die Tabelle vorgegeben wird, kannst du das trotzdem explizit in deinem Model abbilden.
Die Aufrufe sähen dann nur noch so aus: branches.add(current);, und keinerlei Code außer des Repoitorys muss wissen, dass die aus zwei verschiedenen Tabellen kommen.

Wenn du die ID zum Unterscheiden zwischen neuen und bearbeiteten Einträgen nutzt, bietet es sich an, null für neue Einträge zu nutzen, das entspräche dann keiner ID, und nicht einer Magic Number - kann durchaus Fehler unwahrscheinlicher machen.
Und wenn die spezielle ID nur für zwei unterschiedliche Methodenaufrufe des Repos nutzt, kannst du die Unterscheidung dort reinziehen, die ist ja nur für die unterschiedlichen Queres relevant, also nur für Repository-Interna.

Das würde im Client-Code zu diesem: branches.save(current); führen, statt diesem:

if (current.getId() == 0L) {
            branches.add(BranchRepository.INSERT<Location.DOMESTIC>, current); //<- falsch
            //BranchRepository.DOMESTIC geht aber ebenfalls nicht, da ich sonst keine Unterscheidung habe, ob es sich um LOCATION, UPDATE, oder INSERT als EnumMap handelt
        } else {
            branches.update(BranchRepository.INSERT<Location.DOMESTIC>, current); // falsch
        }

 

[maGG]

Danke!

Meinst du, dass ich nur das Enum in Branch ziehen soll, oder das Enum, die EnumMaps und static {}?

Wenn du die ID zum Unterscheiden zwischen neuen und bearbeiteten Einträgen nutzt, bietet es sich an, null für neue Einträge zu nutzen, das entspräche dann keiner ID, und nicht einer Magic Number - kann durchaus Fehler unwahrscheinlicher machen.

Wie kann ich mir das vorstellen? Würdest du mir raten, das Texfeld zum setzen einer Id zu entfernen? Ich dachte halt in der Tabell ist es als Visualisierung gut, da man so direkt weiß, um was es sich handelt.

 

[mrBrown]

Ob es SELECT, UPDATE, oder INSERT ist, ist durch den Methodenaufruf klar

Und wie? Schau dir mal den Code an:
[...]
Dort gibt es keine Unterscheidung. Wenn ich BranchRepository.DOMESTIC setze und meinetwegen Branch.DOMESTIC, wie soll dann zwischen den 3 Optionen unterschieden werden? Mir fällt dazu nur ein, dass ich entweder 3 verschiedenen Enums habe, mit denselben Werten, oder das es beim Einsetzen eine Möglichkeit gibt die richtige EnumMap zu wählen. Durch Rumprobieren kam ich jedoch auf Letzteres nicht, k.A. ob das überhaupt möglich ist. Oder hast du das anders gemeint?

Es gibt aktuell drei Methoden, und jede Methode nutzt nur genau eine der drei Maps:
In add musst du immer INSERT nutzen, in update immer UPDATE und in find (falls die noch so heißt) immer LOCATION (welche man vom Namen her den anderen anpassen sollte).

Du bist niemals an dem Punkt, dass zb add aufgerufen wurde, und du dann noch wissen musst, ob dazu INSERT oder UPDATE gehört - das ist ja schon durch das add klar.

Wie kann ich mir das vorstellen? Würdest du mir raten, das Texfeld zum setzen einer Id zu entfernen? Ich dachte halt in der Tabell ist es als Visualisierung gut, da man so direkt weiß, um was es sich handelt.

Das sichtbare Textfeld hat nur am Rande mit dem Inhalt deines Modells zu tun
Das Textfeld kann ruhig so bleiben, ob es für den Nutzer allerdings klar ist, dass '0' als ID bedeutet, dass ein neuer Eintrag eingefügt wird?

Im Model ist es aber of sinnvoll, nicht 0 als Zeichen für "keine ID" zu nutzen, sondern eben null.
0 ist aus Datenbanksicht in den meisten Fällen eine valide ID, wenn du sehr viel Pech hast, steht in der Datenbank irgendwann ein Datensatz mit ID 0, und deine Anwendung könnte den niemals korrekt behandeln.
null dagegen macht das im Model eindeutiger (null=kein Wert => neuer Eintrag, irgendeine Zahl => bestehender Eintrag), und in der Datenbank uU auch, da NULL keine ID.

 

[maGG]

Achso, also das ich dann anstatt findAll(Location where) habe, dann findAllDomestic() und findAllForeign()?

    public List<Branch> findAll(Location where) throws SQLException{
        String sql = LOCATION.get(where);
        try(PreparedStatement pst = conn.prepareStatement(sql)){
            try(ResultSet rs = pst.executeQuery()){
                List<Branch> result = new ArrayList<>();
                while(rs.next()){
                    result.add(readBranch(rs));
                }
                return result;            
            }
        }
    }
   
    public List<Branch> findAllDomestic() throws SQLException{
        String sql = LOCATION.get(Branch.Location.DOMESTIC);
        try(PreparedStatement pst = conn.prepareStatement(sql)){
            try(ResultSet rs = pst.executeQuery()){
                List<Branch> result = new ArrayList<>();
                while(rs.next()){
                    result.add(readBranch(rs));
                }
                return result;            
            }
        }
    }
   
    public List<Branch> findAllForeign() throws SQLException{
        String sql = LOCATION.get(Branch.Location.FOREIGN);
        try(PreparedStatement pst = conn.prepareStatement(sql)){
            try(ResultSet rs = pst.executeQuery()){
                List<Branch> result = new ArrayList<>();
                while(rs.next()){
                    result.add(readBranch(rs));
                }
                return result;            
            }
        }
    }

So? Gut kann ich auch machen, führt halt dann zu einer Verdoppelung an Methoden in meiner Repository Klasse und ich brauche auch zwei BranchModel Klassen dann, hmm ...

 

[mrBrown]

Achso, also das ich dann anstatt findAll(Location where) habe, dann findAllDomestic() und findAllForeign()?

Nein, wie kommst du darauf?

Ah, dein Beitrag auf den ich geantwortet hab ist mittlerweile völlig anders...

Wenn du dich "aufs Location in Branch ziehen" beziehst:

Branch wird einfach um ein Feld Location location mit Get/Settern ergänzt, die EnumMaps bleiben davon unberührt der Enum auch, außer dass man ihn in das package ziehen kann (sodass er keine "nested class" mehr ist).

findAll bleibt genau so wie bisher: List<Branch> findAll(Location where), du willst ja weiterhin alle zu einer Location.


Die anderen müsstest du in der aktuellen Version mal zeigen, damit man dazu was sagen könnte.
Aber angenommen update sah bisher so aus: void update(Location where, Branch branch) {...}, dann wäre es danach so:

void update(Branch branch) {
   Location where = branch.getLocation();
   //und hier der Rest, der bisher in der Methode stand
}

Mehr Methoden brauchst du damit keinesfalls, es wird nur an anderen Stellen etwas weniger Code.

 

[maGG]

Achso, danke ... das macht jetzt etwas mehr Sinn, bei mir bleibt jetzt aber dennoch eine Frage offen:

    public void updateBranch(Branch branch) throws SQLException{
        Location where = branch.getLocation();
        String sql = UPDATE.get(where);
        try(PreparedStatement pst = conn.prepareStatement(sql)){ 
            pst.setInt(1, branch.getId()); //id
            pst.setString(2, branch.getLabel()); //LABEL
            pst.setString(3, branch.getStrasze()); //STRASZE
            pst.setString(4, branch.getPlzort()); //PLZ_ORT
            pst.setString(5, branch.getOrt()); //ORT
            pst.setInt(6, branch.getVorwahl()); //TEL_LAND
            pst.setLong(7, branch.getTelStartNum()); //TEL_ANFANG_INT
            pst.setString(8, branch.getFaxStartTxt()); //TEL_ANFANG_TXT
            pst.setInt(9, branch.getTelEndDefault()); //TEL_ENDE_DEFAULT
            pst.setString(10, branch.getFaxStartTxt()); //FAX_ANFANG_TXT
            if(branch.getFaxEndTxt().equals("")){
                pst.setObject(11, null);
            }else{
                pst.setString(11, branch.getFaxEndTxt()); //FAX_ENDE_TXT
            }
            pst.setInt(12, branch.getId()); //id
            pst.executeUpdate();
            fireUpdated(branch);
        }   
    }

Mit "Location where = branch.getLocation()" bekomme ich doch jetzt DOMESTIC und FOREIGN zurück oder? Aber ich müsste ja eigentlich einen der beiden Werte setzen, oder?

 

[mihe7]

Aber ich müsste ja eigentlich einen der beiden Werte setzen, oder?

Ja, in findAll()

 

[mrBrown]

Mit "Location where = branch.getLocation()" bekomme ich doch jetzt DOMESTIC und FOREIGN zurück oder?

Ja.

Aber ich müsste ja eigentlich einen der beiden Werte setzen, oder?

Ja. In den find-Methoden und überall da, wo du neue Brasch-Objekte erstellst

 

[mihe7]

Vielleicht noch kurz zur Erklärung: die Location ergibt sich implizit daraus, in welcher Tabelle der Satz steht. Umgekehrt dient die Location eben genau dafür, die Tabelle zu bestimmen, in die eingefügt wird bzw. wo aktualisiert werden muss.

 

[maGG]

Achso ok, das heißt ich müsste mit einer Füllmethode dann die Methode findAll Methode aufrufen, aber ich habe ja ein TabbedPane, wo ich zwischen den Tabellen wechsle... ne ich kapiers noch nicht ganz gut das ihr mein Programm besser versteht als ich

Du hattest mal angesprochen, das es sinn macht, die grafischen Sachen zu trennen. Bei deinem Beispiel hast du in der Main Klasse fast nix drin. Meine Main ist ja rieseig. Jetzt benutze ich den GUI Builder, aber es würde sinn machen zu mindest das zweite Fenster auszulagern, oder? Im Moment lädt das Programm beim Start immer das Hauptfenster und das zweite Fenster zum Editieren. Falls man die DB aber gar nicht editieren möchte, macht es das Programm doch ziemlich langsam, oder? Wäre es dann nicht schlauer, das Ganze auf 2 JFrame Klassen aufzuteilen? Also das Hauptfenster "Main" und meinetwegen "EditData"

 

[mihe7]

Achso ok, das heißt ich müsste mit einer Füllmethode dann die Methode findAll Methode aufrufen, aber ich habe ja ein TabbedPane, wo ich zwischen den Tabellen wechsle... ne ich kapiers noch nicht ganz gut das ihr mein Programm besser versteht als ich

Naja, im Inlandspane rufst Du halt (EDIT: ggf. indirekt) findAll(Location.DOMESTIC) und im Auslandspane findAll(Location.FOREIGN) auf

Du hattest mal angesprochen, das es sinn macht, die grafischen Sachen zu trennen.

Ja. Wir trennen ja schon die ganze Zeit

Jetzt benutze ich den GUI Builder, aber es würde sinn machen zu mindest das zweite Fenster auszulagern, oder?

Ich hab es schon mal angesprochen: Du kannst den GUI-Builder auch dazu verwenden, um einzelne Panels zu designen. Dann landen die Panels in separaten .java-Files.

Beispiel: Du willst einen JFrame mit einem TabbedPane mit zwei Tabs haben. Dann entwirfst Du jeden Tab separat als JPanel (zwei .java-Files) und außerdem den JFrame mit einem TabbedPane. Dan kannst Du, wenn ich mich richtig entsinne, die Panels in das TabbedPane ziehen.

 

[maGG]

Ok, habe mein zweites Fenster jetzt ausgelagert in die Klasse "EditData" und dort jeweils 5 JPanels eingefügt. Ich wollte jetzt mal versuchen eine der Tabellen zu füllen und hab es natürlich erst mal verkackt

Ich bekomme eine NullPointer als Meldung. Hmm was habe ich falsch gemacht? Ich finde das echt mega schwer, diese ganze Verschachtelung ... stimmt wenigstens der Ansatz/Idee in etwa?

Oder brauch ich dafür ein Renderer?

    public EditData() {
        initComponents();
        setLocationRelativeTo(null);
        setResizable(false);
        try {
            br.findAll(Location.DOMESTIC);
        } catch (SQLException ex) {
            ex.printStackTrace();
            JOptionPane.showMessageDialog(null, ex);
        }
        bvd.show();     
    }

    BranchRepository br;
    BranchViewDomestic bvd;
//...

public class BranchViewDomestic {
  
    private final BranchModel branches;
    private Branch current;
    private PanelBranchesDomestic panel;
  
    public BranchViewDomestic(BranchModel branches) {
        this.branches = branches;
    } 

    public void show() {
        branches.show();
        panel.jTableFilialen_Deutschland.setModel(branches);
    }
//...

public class BranchModel extends AbstractTableModel implements TableModel, ListModel<Branch>, ComboBoxModel<Branch>{

    private BranchRepository branches;
    private Branch.Location location;
    private List<Branch> data;
    private Object selected;
  
    public BranchModel(BranchRepository branches, Branch.Location location) {
        this.branches = branches;
        this.location = location;
        try {
            data = branches.findAll(location);
        } catch (SQLException e) {
            data = new ArrayList<>();
            e.printStackTrace();
        }
    }
  
    public BranchModel show() {
        BranchModel bm = new BranchModel(branches, location);
        return bm;
    }
//...

public class BranchRepository {
  
//...
    private final Connection conn;
  
    public BranchRepository() {
        this.conn = null;
    }
  
    public BranchRepository(Connection conn) {
        this.conn = conn;
    }
//... 
    public List<Branch> findAll(Location where) throws SQLException {
        String sql = LOCATION.get(where);
        try(PreparedStatement pst = conn.prepareStatement(sql)) {
            try(ResultSet rs = pst.executeQuery()){
                List<Branch> result = new ArrayList<>();
                while(rs.next()) {
                    result.add(readBranch(rs));
                }
                return result;           
            }
        }
    }
//...

 

[mihe7]

Ich bekomme eine NullPointer als Meldung.

In der Meldung steht genau, an welcher Stelle...

 

[mihe7]

Ich sehe gerade, dass Du es etwas zu kompliziert machst. Meine View-Klassen entsprechen Deinen NetBeans-UI-Builder-Klassen. Das, was Du aktuell in BranchViewDomestic hast, kannst Du direkt in PanelBranchDomestic umsetzen; auf die View-Klasse kannst Du dann verzichten.

 

[maGG]

Ich sehe gerade, dass Du es etwas zu kompliziert machst. Meine View-Klassen entsprechen Deinen NetBeans-UI-Builder-Klassen. Das, was Du aktuell in BranchViewDomestic hast, kannst Du direkt in PanelBranchDomestic umsetzen; auf die View-Klasse kannst Du dann verzichten.

Kannst du das etwas genauer beschreiben?

import ModelClasses.BranchModel;

public class PanelBranchesDomestic extends javax.swing.JPanel {

    public PanelBranchesDomestic() {
        initComponents();
        branches.show(); // hier weiß ich nicht wann überhaupt Location eingesetzt wird, glaube nämlich gar nicht
        jTableFilialen_Deutschland.setModel(branches); 
    }
  
    BranchModel branches;

}

hier bekomme ich wieder eine NullPointerException bei der Zeile, wo mein Kommentar ist -> branches.show()

 

[mihe7]

Deine View-Klasse mappt zwischen Model und Panel. Das kannst Du Dir sparen, wenn Du den relevanten Code einfach direkt in das Panel reinnimmst.

 

[mihe7]

branches.show(); // hier weiß ich nicht wann überhaupt Location eingesetzt wird, glaube nämlich gar nicht

Hier weiß ich gar nicht, was die Zeile überhaupt soll

 

[maGG]

Ah hab gerade gesehen, dass ich BranchModelView anstatt BranchModel hatte, ja naja ich muss doch Werte einsetzen in meine FindAll Methode, daher dachte ich rufe ich den Konstruktor in BranchModel einfach auf, weil da wird ja die Methode aufgerufen. Mit jTableFilialen_Deutschland.setModel(branches); kommt wieder nix raus (NullPointer). Ich muss an irgendeiner stelle die findAll Methode aufrufen und dort muss ich etwas einsetzen, oder?

 

[mihe7]

Das BranchModel ruft die findAll-Methode auf. Im Panel musst Du dann nur das Model setzen und beim Erzeugen des Models gibst Du ja an, für welche Location.

 

[maGG]

meinst du das so?

public PanelBranchesDomestic() {
    initComponents
    branches = new BranchModel(branch, Location.DOMESTIC) // was setze ich für branch ein?
    jTableFilialen_Deutschland.setModel(branches)
}

BranchModel branches;
BranchRepository branch;

was setze ich dann ein? :O

 

[mihe7]

Ein übergebenes BranchRepository

 

[maGG]

ah ich hatte vergessen bei dem Konstruktor den Übergabeparameter Branch zu löschen. Allerdings habe ich immer noch eine Nullpointer Exception, bei der Zeile "data = branches.findAll(location);

//Panel
public PanelBranchesDomestic() {
    branches = new BranchModel(Location.DOMESTIC);
    jTableFilialen_Deutschland.setModel(branches);
}
//BranchModel
public BranchModel(Branch.Location location) {
    this.location = location;
    try {
        data = branches.findAll(location)
    }catch (SQLException e) {
        data = new ArrayList<>();
        e.printStackTrace();
    }
}
//BranchRepository
public List<Branch> findAll(Location where) throws SQLException {
    String sql = LOCATION.get(where);
    try(PreparedStatement pst = conn.preparedStatement(sql)) {
        try (RestultSet rs = pst.executeQuery()) {
            List<Branch> result = new ArrayList<>();
            while (rs.next()) {
                result.add(readBranch(rs));
            }
            return result;
        }
    }
}

Ich glaube es liegt vielleicht an meiner Datenbank Anbindung, ich habe nirgendwo mein Parameter "conn" aus der Hauptklasse angegeben hmm

 

[maGG]

Ein übergebenes BranchRepository

k.A. was das heißen soll, hab jetzt schon eniges ausprobiert, kommt jedes mall Null zurück. Ich brauch einen Wert, den ich dort einsetze.

Wenn du sowas meinst:
Main main;
BranchRepository branch = new BranchRepository(main.conn)

=> geht auch nicht.

 

[mihe7]

Du baust das irgendwie in die falsche Richtung auf.

Ganz "unten" hast Du ein BranchRepository, das muss bei seiner Erzeugung eine Connection bekommen.

Eine Ebene höher liegt das BranchModel, das muss dem entsprechend ein BranchRepository erhalten. Außerdem gibst Du dort noch an, für welche Location das Model gelten soll.

Das BranchModel verwendest Du in Deinem JPanel, also gibst Du es diesem bei seiner Erzeugung mit.

Hier mal ein Schichtenmodell, wobei das UI detailliert, die Anwendung dagegen verkürzt darstellt ist:

+---------------------------------+
| UI-Komponente (z. B. JComboBox) |
+---------------------------------+
                ^
                |
                v
 +---------------------------------+
 |   UI-Model (z. B. BranchModel)  |
 +---------------------------------+
                ^
                |
                v
+------------------------------------+
| Anwendung (z. B. BranchRepository) |
+------------------------------------+
                ^
                |
                v
       +----------------+
       |   Datenbank    |
       +----------------+

 

[maGG]

Ja das sieht schematisch toll aus, vielen Dank, aber ich komme damit nicht weiter leider. Ich habe jetzt 2 Klasse die ein Run Methode haben. Bisher ist meine Methode ConnectDb(); nur in meiner Main Klasse vorhanden. Jetzt habe ich ja mein zweites Fenster ausgelagert, ebenfalls mit einer Run Methode. Bei dieser fehlt jetzt ConnectDb(). Also muss ich irgendwas in dieses bescheuerte Repository Objekt einsetzen und ich habe keine Ahnung was. Das ist wie ein Ratespiel für mich: Schreib 100 verschiedene Sachen rein und schaue obs klappt.

Ich habe jetzt die ConnectDb() Methode auch nochmal in EditData reingeschrieben und in meinem PanelBranchesDomestic habe ich jetzt für das Repository Objekt ein EditData Objekt eingesetzt und darauf eine Getter Methode, um mir Connection conn zu holen. Bringt wieder nix, NullPointer .... gibt bestimmt ne bessere Option, wo ich ConnectDb reinschreiben kann? So schöne diese Verschachtelung auch ist, mittlerweile bin ich echt entnervt davon, es ist mega kompliziert diese Struktur einzuhalten und den Überblick zu behalten wo was eingesetzt wird

 

[mihe7]

Niemand hat gesagt, dass es einfach wird - wobei das Prinzip an sich sogar sehr einfach ist:

Wenn Du den Auftrag bekommst, ein Loch zu bohren, dann fängst Du nicht an, eine Bohrmaschine zu bauen. Du lässt Dir auch nicht die Einzelteile geben, um sie dann zu einer Bohrmaschine zusammenzusetzen. Alles, was Du brauchst, ist eine Bohrmaschine, die und nur die musst Du bekommen. Job done.

Wenn ein Objekt ein anderes erzeugt, hat dieses Objekt dafür zu sorgen, dass das erzeugte Objekt mit allem ausgestattet wird, was es braucht.

Genau diesem Prinzip versucht man, in der Objektorientierung zu folgen. Wenn Du z. B. Swing ansiehst, dann hast Du eine ganz klare Trennung zwischen der UI-Komponente (JComboBox) und dem UI-Model (ComboBoxModel). Wenn Du in Deiner Panel-Klasse ein JComboBox-Objekt erzeugst, dann hat Dein Panel-Objekt dafür zu sorgen, dass das JComboBox-Objekt das benötigte ComboBoxModel erhält. Daher gibst Du der JComboBox das Model mit - was in dem Model steckt, interessiert die JComboBox nicht. Das ist keine Erfindung von mir, um Dich zu ärgern, sondern vom Framework so vorgegeben

Das gleiche machen wir nun einfach weiter: wir haben eine ComboBoxModel-Implementierung (BranchModel). Das BranchModel braucht ein BranchRepository. Woraus das BranchRepository besteht, interessiert das BranchModel nicht. Wichtig ist für das BranchModel in erster Linie findAll() und ggf. noch die Möglichkeit, das Repository zu beobachten (addRepositoryListener).


Bis dahin ist die Sache recht einfach. Wenn Du ein BranchRepository-Objekt hast, kannst Du ganz einfach ein BranchModel erzeugen. Dieses kannst Du verwenden, um es an eine JComboBox zu übergeben. Die JComboBox kannst Du verwenden, um es in ein JPanel einzufügen. An der Stelle hast Du einen kleinen Bruch, der durch den UI-Builder entsteht: um ein Panel zu erzeugen, fügt der UI-Builder nicht einfach Komponenten einem JPanel hinzu (das wäre das Prinzip wie oben beschrieben), sondern der UI-Builder erstellt eine eigene Klasse für das Panel und in dieser Klasse wird jetzt selbst die JComboBox erstellt (das wäre wie Bohrmaschine selber bauen).

Daher gibt es nun zwei Möglichkeiten:
a) Du übergibst das BranchModel an das JPanel
b) Du erzeugst auch das BranchModel im JPanel

Da die JComboBox und das BranchModel im UI sehr eng zusammenhängen (die von JPanel abgeleitete Klasse ist ja gerade dazu da, z. B. die Inlands-Filialen anzuzeigen), wäre b) durchaus ein gangbarer Weg. Um das BranchModel im JPanel zu erzeugen, brauchst Du aber ein BranchRepository. Dieses gibst Du dem JPanel mit (Konstruktor).

Bleibt die Frage: wo kommt das BranchRepository her? Ganz einfach: von der Klasse, die das JPanel erzeugt. Das setzt sich ggf. bis zur main-Methode durch.

Im Idealfall gibt es genau eine einzige Instanz von BranchRepository. Dann nämlich kann jedes x-beliebige Objekt (Fenster, Panel, ComboBox, Tabelle, ...) über Änderungen informiert werden, unabhängig davon, wer die Änderung durchführt.

Du kannst also beim Start der Anwendung die DB-Verbindung herstellen, ein BranchRepository-Objekt erzeugen (erhält dann die gerade erstellte Connection) und dieses gibst Du Deinem Fenster/JPanel mit.

 

[maGG]

Vielen Dank für die ausführliche Erklärung! Mein Code müsste dann eigenlich schon funktionierten, im Moment kommt aber immer noch NullPointer raus:

public PanelBranchesDomestic() {
    intiComponents();
    branches = new BranchModel(branch, Location.DOMESTIC)
    jTableFilialen_Deutschland.setModel(branches);
}

EditData conn;
BranchModel branches;
BranchRepository branch = new BranchRepository (conn.getConn());

Könntet ihr mir wenigstens sagen, ob der Code soweit stimmt?

 

[mrBrown]

Könntet ihr mir wenigstens sagen, ob der Code soweit stimmt?

Naja, er funktioniert nicht, oder?



Lösch aus der Klasse das Feld conn, und ergänz den Konstruktor um einen Parameter BranchRepository und übergib das dort, wo du das Panel erstellst, so wie @mihe7 das auch schon sagte:

Du kannst also beim Start der Anwendung die DB-Verbindung herstellen, ein BranchRepository-Objekt erzeugen (erhält dann die gerade erstellte Connection) und dieses gibst Du Deinem Fenster/JPanel mit.

 

[maGG]

Lösch aus der Klasse das Feld conn

aus der Klasse PanelBranchDomestic?

ergänz den Konstruktor um einen Parameter BranchRepository

so?

public PanelBranchesDomestic (BranchRepository branchRepo) {
    initComponents();
    this.branchRepo = branchRepo;   
}

übergib das dort, wo du das Panel erstellst

also in meiner Klasse EditData? Was meinst du mit übergeben? was auf was?
Mit den Hinweisen von euch verstehe ich es leider nicht, wie ich es bei mir umsetzen kann.

Im Moment verstehe ich euch so (so ist es aber falsch):

public EditData() {
    initComponents();
    setLocationRelativeTo(null);
    setResizable(false);
    ConnectDb();
    panel = new PanelBranchesDomestic (branch);
    branch = new BranchRepository (conn)
    branches = new BranchModel (branch, Location.DOMESTIC);
    panel.jTableFilialen_Deutschland.setModel(branches);
}

Connection conn;
BranchRepository branch;
BranchModel branches;
PanelBranchesDomestic panel;

 

[mrBrown]

panel = new PanelBranchesDomestic (branch);
branch = new BranchRepository (conn)

Du solltest dir die Reihenfolge noch mal angucken...