[Java] Inserimento dati in un DB

[Jamie04]

- Il codice è tutto ammassato, non ci sono righe vuote che separano le varie operazioni. Ad esempio, hai codice di controllo e operazioni vere e proprie troppo amalgamate. Questo rende difficile capire a colpo d'occhio cosa stai facendo
- Usi variabili inutili, ad esempio per la query: passa direttamente al prepareStatement, no?

Ok, ho messo un po' di spazi e di commenti in più e tolto la stringa inutile

Non c'è il finally: in caso di eccezione lo statement resta aperto, andando a occupare inutilmente risorse.
Il problema più grave IMHO è che in caso di eccezione stai facendo <i>troppo</i>: insertMenu dovrebbe fare una query, e basta. Te gli stai dando la responsabilità di avvisare l'utente che qualcosa è andato storto. Questa gestione andarebbe fatta a livello più alto: cosa succede se insertMenu facesse parte di una libreria che distribuisci ad altri sviluppatori? Potrebbe essere usata in un contesto in cui l'interfaccia grafica neanche esiste (pensa ad un batch notturno che legge dei dati da file e inserisce tutto a db, senza l'intervento dell'operatore).

Ok, però questo è solo un progetto per un esame, non è un vero software che dovrò distribuire a qualcuno. Non è previsto che debba funzionare senza interfaccia grafica perciò non mi sono preoccupata di questi dettagli. Le tue critiche mi sembrano più che sensate ma credo che nel contesto in cui sono attualmente posso evitarmi queste complicazioni. Del resto nel corso che abbiamo fatto nessuno ci ha spiegato veramente come mettere in piedi un'applicazione complessa.
Ammetto che la gestione delle eccezioni è ancora un po' oscura per me, infatti mi limito a prendere spunto da esempi già fatti. Ho capito il concetto a grandi linee ma non conosco tutte le sottigliezze.

Sarebbe un problema se venisse riassegnato visto che in genere un auto_increment è usato (purtroppo) come primary key...

perché purtroppo?

Ah, anche il fatto che sia static non mi piace ma bisognerebbe vedere un attimo come hai organizzato l'architettura dell'applicazione.

perché non dovrebbe essere static? sinceramente su questa cosa non ci ho riflettuto molto mi sono limitata a modificare la classe che ci aveva fornito il prof a suo tempo per connettersi a un database e i metodi erano tutti statici, così gli altri che ho inserito (inserimento dati e creazione db e tabelle) li ho fatti uguale...

grazie per i tuoi consigli!

[Jamie04]

Ho modificato il metodo così:

    public static void insertMenu(String field) throws SQLException {
        
        PreparedStatement statement = null;
        
//verifico che l'utente non abbia inserito un campo vuoto
        try {
            if (field == null || field.isEmpty()) {
                JOptionPane.showMessageDialog(null, "Non puoi inserire un menu senza nome!", "", JOptionPane.WARNING_MESSAGE);
                return;
            }

//inserisco fisicamente nel db il nuovo menu
            statement = connection.prepareStatement("INSERT INTO Menu VALUES (NULL, ?)");
            statement.setString(1, field);
            statement.executeUpdate();
            
//catturo le eccezioni e chiudo lo statement per liberare risorse
        } catch (SQLException e) {
            JOptionPane.showMessageDialog(null, "Si è verificato un problema\nnell'inserimento dei dati!", "", JOptionPane.WARNING_MESSAGE);
            Logger.getLogger(DBConnection.class.getName()).log(Level.SEVERE, null, e);
        } finally {
            if (statement != null) {
                statement.close();
            }
        }
    }

Però adesso nella classe dove lo chiamo, con questa istruzione:

DBConnection.insertMenu(field);

Mi dice: unreported exception.
Come mai?

[LeleFT]

Ho modificato il titolo della discussione visto che, a partire dal secondo messaggio, l'argomento è radicalmente cambiato e non ha più a che fare con le JList.

In futuro, se il problema cambia, apri una nuova discussione ed eventualmente linka quella originale (se serve per avere informazioni dettagliate che sono in comune o se serve per "riprendere il filo del discorso". Ma non continuare con un problema diverso nella stessa discussione, altrimenti diventa un macello.


Ciao.

[Jamie04]

hai ragione, chiedo scusa

[schumy2000]

Posso chiederti una cosa?
Perché fai il try catch di SQLException e contemporaneamente fai il throws di SQLException portandoti al livello superiore (livello del chiamante) l'eccezione stessa?

public static void insertMenu(String field) throws SQLException {
    PreparedStatement statement = null;
    try {
        if (field == null || field.isEmpty()) {
            JOptionPane.showMessageDialog(null, "Non puoi inserire un menu senza nome!", "", JOptionPane.WARNING_MESSAGE);
            return;
        }


        statement = connection.prepareStatement("INSERT INTO Menu VALUES (NULL, ?)");
        statement.setString(1, field);
        statement.executeUpdate();


    } 
    catch (SQLException e) {
        JOptionPane.showMessageDialog(null, "Si è verificato un problema\nnell'inserimento dei dati!", "", JOptionPane.WARNING_MESSAGE);
        Logger.getLogger(DBConnection.class.getName()).log(Level.SEVERE, null, e);
    } 
    finally {
        if (statement != null) {
            statement.close();
        }
    }
}

O fai il try catch o fai il throws...entrambi non ha alcun senso se non scrivere righe di codice in più

Ciao.

[Alex'87]

Posso chiederti una cosa?
Perché fai il try catch di SQLException e contemporaneamente fai il throws di SQLException portandoti al livello superiore (livello del chiamante) l'eccezione stessa?

O fai il try catch o fai il throws...entrambi non ha alcun senso se non scrivere righe di codice in più

Ciao.

La close() nel finally lancia SQLException... Io non metterei proprio il catch (al resto risposto stasera, ora sono incasinato)

[Jamie04]

Infatti il problema è che anche il finally può generare un'eccezione e il compilatore mi stava avvertendo proprio di questo.
Ho modificato così il metodo e ora funziona:

    public static void insertMenu(String field) {
        
        PreparedStatement statement = null;
        
//verifico che l'utente non abbia inserito un campo vuoto
        try {
            if (field == null || field.isEmpty()) {
                JOptionPane.showMessageDialog(blabla);
                return;
            }

//inserisco fisicamente nel db il nuovo menu
            statement = connection.prepareStatement("INSERT INTO Menu VALUES (NULL, ?)");
            statement.setString(1, field);
            statement.executeUpdate();
            
//catturo le eccezioni e chiudo lo statement per liberare risorse
        } catch (SQLException e) {
            JOptionPane.showMessageDialog(blabla);
            Logger.getLogger(DBConnection.class.getName()).log(Level.SEVERE, null, e);
        } finally {
            if (statement != null) {
                try {
                    statement.close();
                } catch (SQLException ex) {
                    Logger.getLogger(DBConnection.class.getName()).log(Level.SEVERE, null, ex);
                }
            }
        }
    }

Altrimenti avrei dovuto aggiungere il try-catch relativo al finally nella classe dove chiamavo il metodo, e non mi sembrava granché come soluzione.
Adesso ho il problema che non so come aggiornare il contenuto della JList dopo che ho inserito dei dati nuovi nel db