Problema con variabile volatile

[andbin]

Voglio essere sincero e onesto: il tuo codice è parecchio "fumoso" e ci sono svariate cose che sono inappropriate e per niente belle.
Il problema non è una variabile volatile ......

[linux_r]

Voglio essere sincero e onesto: il tuo codice è parecchio "fumoso" e ci sono svariate cose che sono inappropriate e per niente belle.
Il problema non è una variabile volatile ......

ti sarei grato se specificassi cosa non ti piace o non va

[andbin]

ti sarei grato se specificassi cosa non ti piace o non va

Oh certo ... ma l'elenco è lungo (e non sarò sicuramente esaustivo al 100%):

- il codice non è completo ma dai metodi si deduce che SWListaOperatori è uno SwingWorker. SWListaOperatori contiene una inner-class SWInviaAppuntamento che è di nuovo uno SwingWorker (di per sé tecnicamente nulla di sbagliato ma è perlomeno strano/inusuale). Nel doInBackground di SWInviaAppuntamento c'è una anonymous inner class per un Runnable. Nel suo run() ad un certo punto c'è una ulteriore anonymous inner-class per un WindowAdapter. Quando si arriva ad un tale livello di annidamento con pure tutti i vari blocchi if/try in mezzo, dovrebbe suonarti un campanello di allarme .... un grosso campanello ...

- in questo codice usi di tutto e di più: da SwingWorker, ai thread, alla sincronizzazione con synchronized, all'uso di volatile, all'uso della condition-queue "intrinseca" (il wait/notify sugli oggetti). Dubito che quello sia l'unico modo per fare quello che volevi ma anche se fosse davvero tutto necessario, così è comunque un po' sparpagliato e fumoso.

- nel primo doInBackground non si capisce assolutamente che cosa si ottiene. Cosa conterrà quel ArrayList<Object>? Che colonna (e di che tipo) prendi con getObject(1) ? Solo tu lo sai ....
Manca di chiarezza.

- sempre in questo doInBackground il close non viene fatto sempre. Se next() o getObject() lanciano SQLException, tu non chiudi nulla. Manca di correttezza nella gestione delle eccezioni.
E anche se facessi tutto corretto con le eccezioni, tutta la logica di query ed estrazione dal result-set andrebbe fatta in una classe separata.

- guardando le invocazioni sulla istanza di Database, sembrano un po' una accozzaglia di operazioni ben diverse. Evita di fare una classe "Database" che fa di tutto e di più. Avresti potuto/dovuto applicare almeno il pattern DAO, per risolvere anche quanto detto al punto precedente.

- Database.getInstance().updateField(10, 1, Integer.toString(cliente.getId_cliente()), (String) userOperatore, GestoreDatabase.Tabella.CLIENTE);
Non è un buon modo di fare un update di uno o più campi. Tra l'altro "sembra" un metodo molto generico/arbitrario e quindi poco comprensibile. Cosa sono ad esempio 10 e 1 ? Solo tu lo sai ....

- nel run() del tuo thread vai ad accedere alla interfaccia utente. Non sei nel contesto del EDT, tutto quello che fai con JOptionPane, setVisible(true) ecc... è tutto inappropriato.

- nel run() comunque c'è un mix di accesso a database, interfaccia grafica, wait/notify. Tutto potenzialmente molto dubbio (fumoso sicuramente).

- fai un synchronized su schedaCliente che è un oggetto GUI. Per quanto ne so, non c'è nulla di intrinsecamente (e tecnicamente) errato nell'usare oggetti GUI come "lock". Ma personalmente io non lo farei mai nemmeno se mi pagate il doppio del mio stipendio ....

E mi sono limitato solo alle cose più evidenti ....

[linux_r]

Database.getInstance().updateField(10, 1, Integer.toString(cliente.getId_cliente()), (String) userOperatore, GestoreDatabase.Tabella.CLIENTE);
Non è un buon modo di fare un update di uno o più campi. Tra l'altro "sembra" un metodo molto generico/arbitrario e quindi poco comprensibile. Cosa sono ad esempio 10 e 1 ? Solo tu lo sai ....

Per quanto riguarda la classe database al suo interno i metodi sono commentati e hanno la javadoc, comunque si è un metodo generico e a dire il vero non ci vedo nulla di male null'utilizzarlo.

- guardando le invocazioni sulla istanza di Database, sembrano un po' una accozzaglia di operazioni ben diverse. Evita di fare una classe "Database" che fa di tutto e di più. Avresti potuto/dovuto applicare almeno il pattern DAO, per risolvere anche quanto detto al punto precedente.

grazie per questo consiglio , purtroppo è un corso introduttivo a java e non sapevo nemmeno l'esistenza di questo pattern, non è tra gli argomenti del corso , almeno adesso so che esiste

- il codice non è completo ma dai metodi si deduce che SWListaOperatori è uno SwingWorker. SWListaOperatori contiene una inner-class SWInviaAppuntamento che è di nuovo uno SwingWorker (di per sé tecnicamente nulla di sbagliato ma è perlomeno strano/inusuale). Nel doInBackground di SWInviaAppuntamento c'è una anonymous inner class per un Runnable. Nel suo run() ad un certo punto c'è una ulteriore anonymous inner-class per un WindowAdapter. Quando si arriva ad un tale livello di annidamento con pure tutti i vari blocchi if/try in mezzo, dovrebbe suonarti un campanello di allarme .... un grosso campanello ..

Non mi pare di aver letto da nessuna parte che non si possano annidare classi oltre un certo livello, e' una regola?

sempre in questo doInBackground il close non viene fatto sempre. Se next() o getObject() lanciano SQLException, tu non chiudi nulla. Manca di correttezza nella gestione delle eccezioni.
E anche se facessi tutto corretto con le eccezioni, tutta la logica di query ed estrazione dal result-set andrebbe fatta in una classe separata.

mmmm su questo sono d'accordo con te dovrei farlo stesso nella classe database o creare una classe a parte?

- nel primo doInBackground non si capisce assolutamente che cosa si ottiene. Cosa conterrà quel ArrayList<Object>? Che colonna (e di che tipo) prendi con getObject(1) ? Solo tu lo sai ....
Manca di chiarezza.

Anche in questo caso il metodo è commentato e spiega cosa contiene quell'array di ritorno

Comunque le critiche purchè costruttive sono sempre accettatissime

[andbin]

comunque si è un metodo generico e a dire il vero non ci vedo nulla di male null'utilizzarlo.

No, non è comunque un "buon" modo.

Non mi pare di aver letto da nessuna parte che non si possano annidare classi oltre un certo livello, e' una regola?

Sì ... del "buon senso".

Anche in questo caso il metodo è commentato e spiega cosa contiene quell'array di ritorno

Ammetto che quando ho letto il tuo codice, mi sono concentrato sul codice ... e non sui commenti.
Ma se sono nomi .... allora perchè "Object" ?

[linux_r]

perchè magari un giorno sarebbe potuto cambiare il campo del database , ho messo object per non dover poi modificare il codice

[Alex'87]

perchè magari un giorno sarebbe potuto cambiare il campo del database , ho messo object per non dover poi modificare il codice

Allora metti un generico T, no?

[linux_r]

giacchè comuqnue l'argomento si è spostato sulla programmazione java vorrei chiederti una cosa. "uno stream è un flusso di dati, un oggetto dal quale si può leggere una sequenza di byte prende nome di input stream , mentre un oggetto nel quale si può scrivere una sequenza di byte prende nome di output stream o sink stream. Vengono detti nodi di stream i file , le memorie e pipe tra thread o processi. Source stream e sink stream sono entrambi dei node streams-."
Cosa ne pensi di questa definizione ? la trovi corretta?