Discussion :

[snyler]

Bonjour,

Je termine actuellement mon BTS informatique option développement et pour mon projet de fin d'année j'ai réalisé une application de facturation en Java. Celle-ci permet la gestion de factures et de prestations via une IHM et l'utilisation de tableaux grâce à des modèles redéfinis. Voilà pour l'aperçu de l'application.

Si vous aviez le temps et la gentillesse de regarder mon code j'aimerais avoir des retours afin de savoir si mon code est tout de même propre, si j'ai bien assimilé la notion d'objet car je constate que je fais souvent directement

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

 new Classe();

et très rarement

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

 Classe maClasse = new Classe();

ce qui me pousse à me demander si j'ai bien compris la notion de langage orienté objet.

D'un autre côté, est-ce que mes variables sont bien encapsulées et est-ce que, à force de vouloir découper, je ne découpe peut être pas trop en classes ?

Voilà mes principales interrogations, si vous avez la moindre remarque à faire n'hésitez pas, mon but est de comprendre mes erreurs afin de ne plus les refaire.

La pièce jointe contient le projet Java, la base de données SQL ainsi que les procédures et triggers. Je n'ai pas mis l'API utilisée pour la gestion des PDF car cela faisait un fichier trop volumineux.

Merci pour votre aide

[eulbobo]

J'ai regardé un peu, et effectivement, ça ressemble plus à de la programmation séquentielle/évenementielle qu'à de la programmation orientée objet. Chaque fois que tu fais un new (Erreur, Menu, etc...), chaque instance ainsi créée n'a aucun lien avec les autres (sauf pour les éléments statiques et publiques, ce qui est dangereux)!
Chaque classe gère elle-même son propre affichage.

Un objet, c'est ce que tu vois devant toi.
Une tasse, c'est un Objet. La tasse a des propriétés, comme la taille, le poids, la couleur... La contenance parce que c'est un Contenant (qui peut faire l'objet d'une classe générique ou d'une interface).
Un téléphone c'est un objet, avec des propriétés comme la taille, le poids, la couleur... Il a des Touches (qui peuvent aussi faire l'objet d'une classe définissant, la forme, la couleur, l'effet...)
Une facture papier, c'est un objet, avec des valeurs renseignées comme la date, le numéro, et des tableaux contenant des Lignes, et pour chaque Ligne il y a des propriétés spécifiques


En soit, ta solution, si elle marche c'est tant mieux mais tu perds la notion de responsabilité qui est une des bases de la programmation orientée objet : quand tu crées une classe, tu dois tendre à ce qu'elle n'ait qu'une seule responsabilité. Ici, chacune gère l'affichage, la logique applicative, la gestion des erreurs et la navigation (ou les évènements), le requétage...

Par exemple, ta Classe "Requete" a l'air d'avoir en charge toutes les requêtes de ton application, avec des duplications de codes. Si ta classe s'appelle Requete, qu'elle se charge effectivement d'exécuter les requêtes... Et uniquement ça ! Pas des requêtes spécifiques à des parties du "métier"




Essayes de redécouper ton application en 3 "couches".
Une première couche à laquelle tu dédie le requêtage/récupération de données depuis ta base de données
Une deuxième dans laquelle tu gère la logique applicative
Une troisième dans laquelle tu ne gère QUE l'affichage.
Pour faire court sur ce qu'il faut que tu fasse :
- Dans facture, découpe la partie affichage de la partie récupération de données. Au passage, essayes de créer un Objet qui représente une facture (à savoir, un numéro, des lignes de factures, ...), un Objet qui représente UNE Ligne de facture (vu que tu as besoin d'une liste de lignes de factures), ... Bref, crée des structures qui correspondent à ce que tu manipules comme si tu avais du papier. Tu ne manipules pas que des morceaux de papiers, tu manipules une grande feuille sur laquelle il y a des informations
- Utilise tes objets imbriqués les uns avec les autres pour gérer l'affichage (avec une classe qui se chargera d'afficher spécifiquement une facture... Ou une liste de factures...)

Sinon, petits trucs :
- une connexion à une base de données, ça se ferme toujours ! Ou il faut utiliser un pool de connexion. Si tu laisses des connexions ouverte, tu risques de ne plus pouvoir te connecter à ta base au bout d'un moment.
- ton objet connexion est un faux singleton: si quelqu'un refait un new ConnexionBDD à un moment, tu perds tout (vu que ton lien vers la connexion est déclaré en statique, donc partagée par toutes les instances de connexion) ! Si tu ne veux qu'une seule connexion, il faut que tu implémentes correctement ton singleton.
- si un élément dépend d'un autre (le menu par rapport au panel principal), il doit avoir une référence pour y être lié. Un menu n'a pas de sens s'il n'est pas lié à un Panel.
- de la même façon, si c'est le panel principal qui doit gérer les erreurs, c'est lui qui doit les récupérer : faire une méthode afficherErreur(String text) dans ton panel principal, et appeler cette méthode partout quand tu as une erreur (en passant le bon message d'erreur)
- MySQLIntegrityConstraintViolationException : on ne doit pas gérer les cas testables en utilisant les Exceptions. Les exceptions sont des défauts non prévus/prévisible qui arrivent à l'exécution. Un doublon en base, c'est prévisible avec une simple requête ! (accessoirement, si tu passes à Postgres derrière, ton code ne marchera plus)

[snyler]

Tout d'abord, merci pour ta longue réponse !

J'ai regardé un peu, et effectivement, ça ressemble plus à de la programmation séquentielle/évenementielle qu'à de la programmation orientée objet. Chaque fois que tu fais un new (Erreur, Menu, etc...), chaque instance ainsi créée n'a aucun lien avec les autres (sauf pour les éléments statiques et publiques, ce qui est dangereux)!

Il me semblait bien que je n'exploitais pas bien les possibilités offertes par le langage objet en faisant simplement des "New Classe()". Je vais donc essayer de créer des objets et de retravailler dessus sans faire de New à chaque fois.

En soit, ta solution, si elle marche c'est tant mieux mais tu perds la notion de responsabilité qui est une des bases de la programmation orientée objet : quand tu crées une classe, tu dois tendre à ce qu'elle n'ait qu'une seule responsabilité. Ici, chacune gère l'affichage, la logique applicative, la gestion des erreurs et la navigation (ou les évènements), le requétage...

J'ai essayé justement de découper chacune des "pages" de mon application dans une classe spécifique. L'affichage de chaque "page" est gérée dans la classe en question et pour tout ce qui est requête je fais ensuite appel à un objet de type "Requête" depuis lequel j'appelle la méthode à qui je transmets la requête à effectuer. Si j'ai bien compris tu me dis donc que la solution que j'ai utilisé est inadaptée alors que j'ai justement essayé de faire ce que tu dis ici

Par exemple, ta Classe "Requete" a l'air d'avoir en charge toutes les requêtes de ton application, avec des duplications de codes. Si ta classe s'appelle Requete, qu'elle se charge effectivement d'exécuter les requêtes... Et uniquement ça ! Pas des requêtes spécifiques à des parties du "métier"

Ai-je donc mal compris ce que tu m'as expliqué ?

Dans facture, découpe la partie affichage de la partie récupération de données

C'est ce que j'ai justement voulu faire en créant une classe "PageFacture" et une classe "AfficheFacture" où "AfficheFacture" est chargé de récupérer les données depuis la base de données (en utilisant lui aussi un objet de type "Requete") puis de les mettre dans mon JTable. Ma classe "PageFacture" utilise alors un objet de type "AfficheFacture". Je pense néanmoins que tu parles d'une autre façon de découper mais je ne comprends pas laquelle.

- de la même façon, si c'est le panel principal qui doit gérer les erreurs, c'est lui qui doit les récupérer : faire une méthode afficherErreur(String text) dans ton panel principal, et appeler cette méthode partout quand tu as une erreur (en passant le bon message d'erreur)

La classe "Erreur" que j'ai crée ayant pour constructeur

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

public Erreur(String titre, String message)

ne fait pas l'affaire ? J'appelle cette classe dès que je rencontre une erreur. Cette classe crée un JDialog modal qui affiche justement l'erreur en question.

- MySQLIntegrityConstraintViolationException : on ne doit pas gérer les cas testables en utilisant les Exceptions. Les exceptions sont des défauts non prévus/prévisible qui arrivent à l'exécution. Un doublon en base, c'est prévisible avec une simple requête ! (accessoirement, si tu passes à Postgres derrière, ton code ne marchera plus)

C'est vrai, j'avoue ne pas y avoir pensé, je me suis dis "Cool ils proposent de gérer les doublons de clés primaires" sans chercher plus loin !

Cela fait beaucoup de questions et j'en ai conscience mais j'aimerais avoir le maximum de bonnes habitudes afin de bien apprendre à utiliser les possibilités et les normes du langage orienté objet.

[eulbobo]

Pour ton objet Requete, le problème c'est qu'il se charge de faire des requêtes mais il renvoie des éléments de requête (ResultSet) : c'est à éviter si tu ne veux pas avoir de problèmes de connexion à ta base de données (avec des ressources non fermées)
En gros, ton objet requête fait bien des requêtes, mais il ne va pas au bout de sa fonction : renvoyer des résultats. Pour l'instant, c'est une simple classe de type Helper qui ne fait que renvoyer le résultat brut de la requête. Pour l'améliorer tu dois faire en sorte que les objets ResultSet et Statement soient créés et fermés au sein des méthodes de Requete et n'en sortent jamais ! (et faut les fermer ensuite).

Là tu te dis "oui, mais une fois que je récupère mon ResultSet, il faut bien que je puisse avoir un moyen de le transformer en un résultat qui ait un sens".
Du coup, plusieurs possibilités :
- Faire en sorte que ta classe Requete soit abstraite, déclarer une méthode abstraite de type protected qui s'appellera par exemple "convertResultSet", et ensuite, pour chaque utilisation de requête dont tu as besoin, créer une implémentation de Requete (RequeteFacture par exemple) qui ne fera qu'implémenter la conversion de ResultSet

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
 
public abstract class Requete {
		/**
                 * Exécute une requête de type select et la renvoie sous forme de ResultSet
                 * @param requete
                 * @return result
                 */	
public List requeteSelect(String requete){
		List result = new ArrayList();
		Connection connexion = ConnexionBDD.getInstance();// On récupère l'instance de notre connexion
		try (Statement state = connexion.createStatement();
			ResultSet rs = state.executeQuery(requete); ) {
 
			result = convertResultSet(rs);
		} catch(Exception e){
			e.printStackTrace();
		}
 
		return result;
	}
 
	protected abstract List convertResultSet(ResultSet rs);
 
//Autres méthodes
}

Ici je considère que tu es au moins en Java7, la notation try (Statement state = connexion.createStatement(); ResultSet rs = state.executeQuery(requete); ) permettant de fermer les ressources Statement et ResultSet dans l'ordre inverse de leur déclaration (donc d'abord rs, puis state) à la fin de l'exécution.

Ensuite, tu peux encore améliorer les choses en déclarant un type générique à ta classe abstraite, ce qui te permettra d'être sûr de récupérer les bonnes instances d'objet dans ta liste

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
 
public abstract class Requete<T> {
		/**
                 * Exécute une requête de type select et la renvoie sous forme de ResultSet
                 * @param requete
                 * @return result
                 */	
public List<T> requeteSelect(String requete){
		List<T> result = new ArrayList<T>();
		Connection connexion = ConnexionBDD.getInstance();// On récupère l'instance de notre connexion
		try (Statement state = connexion.createStatement();
			ResultSet rs = state.executeQuery(requete); ) {
 
			result = convertResultSet(rs);
		} catch(Exception e){
			e.printStackTrace();
		}
 
		return result;
	}
 
	protected abstract List<T> convertResultSet(ResultSet rs);
 
//Autres méthodes
}

Et ton implémentation pour facture qui est alors

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
 
public class RequeteFacture extends Requete<Facture> {
 
	protected List<Facture> convertResultSet(ResultSet rs){
		// code pour transformer ton resultSet en liste de factures
	}
}

A l'utilisation, tu n'auras qu'à appeler la méthode comme ça :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
 
RequeteFacture rqFact = new RequeteFacture();
List<Facture> listFactures = rqFact.requeteSelect("select...");

Inconvénient de ce code : ta méthode de conversion dépend de la requête. Mais d'ailleurs, pourquoi les REQUETES sql ne sont PAS dans la classe REQUETE ?

Modifions ça un petit peu...
Dans ta classe PageAjoutFacture par exemple, tu fais la requête suivante :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
 
ResultSet resultat = req.requeteSelect("select intitule from ligue");

Et si dans la classe RequeteFacture on rajoutait une méthode?

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
 
public class RequeteFacture extends Requete<Facture> {
 
public List<String> getIntitulesLigne() {
		List<String> result = new ArrayList<String>();
		Connection connexion = ConnexionBDD.getInstance();
		try (Statement state = connexion.createStatement();
				ResultSet rs = state.executeQuery("select intitule from ligue");){
			while (rs.next()){
				result.add(rs.getString(1));
			}
		} catch (Exception e){
			e.printStackTrace();
		}
 
		return result;
	}
 
	protected List<Facture> convertResultSet(ResultSet rs){
		// code pour transformer ton resultSet en liste de factures
	}
}

Ici, tu disposes d'une classe fille dont le rôle est d'interroger la base de données pour un domaine précis et de renvoyer des résultats. Et le fait de renvoyer des intitulés de lignes est propre au traitement d'une facture, ça n'a rien de générique (donc rien à faire dans une classe de niveau plus élevé)
Avantage : le code SQL est regroupé dans les classes qui gèrent les accès et les requêtes à la base de données. Du coup, les interfaces n'ont plus à devoir manipuler des objets du niveau de la base de données (baisse du niveau de couplage)

Dernière amélioration possible ici : utiliser le concept de composition d'objets avec des classes qu'on va appeler Transformer. Dont le but sera de transformer un ResultSet précis dans un résultat.

Reprenons le code et ajoutons ça
Le Transformer

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
 
public interface Transformer<T>{
 
         // transformer une ligne de resultSet en un objet
         T transformRs(ResultSet rs) throws SQLException;
}

Dans Requete

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
 
public abstract class Requete<T> {
		/**
                 * Exécute une requête de type select et la renvoie sous forme de ResultSet
                 * @param requete
                 * @return result
                 */	
protected List<T> requeteSelect(String requete, Transformer<T> tr){// passage en protected, on ne l'appelle plus directement
		List<T> result = new ArrayList<T>();
		Connection connexion = ConnexionBDD.getInstance();// On récupère l'instance de notre connexion
		try (Statement state = connexion.createStatement();
			ResultSet rs = state.executeQuery(requete); ) {
 
			result = convertResultSet(rs, tr);
		} catch(Exception e){
			e.printStackTrace();
		}
 
		return result;
	}
 
        // méthode de conversion générique
	protected List<T> convertResultSet(ResultSet rs, Transformer<T> tr) throws SQLException{
		List<T> result = new ArrayList<T>	();
		while (rs.next()){
			result.add(tr.transformRs(rs));
		}
		return result;
        }
 
//Autres méthodes
}

Et dans RequeteFacture, ça devient

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
public class RequeteFacture extends Requete<Facture> {
 
	public List<String> getIntitulesLigne() {
		// création de ton transformer, ici en anonymous inner class, mais on peut faire des classes réelles aussi
		Transformer<String> stringTransformer = new Transformer<String>(){
			String transformRs(ResultSet rs)throws SQLException{
				return rs.getString(1);
			}
		};
                // ici, tu appelles la méthode de la classe mère qui exécuter la requête, et qui transforme les résultats
                return requeteSelect("select intitule from ligue", stringTransformer );
	}
}

Du coup, on a quoi maintenant?
Une classe Requete abstraite qui fait des requêtes mais qui seule n'a pas de sens (il faut implémenter les requêtes)
Des classes spécifiques pour faire des requêtes spécifiques : on peut se concentrer sur la requête SQL et sa transformation en Objets via des classes spécifiques (Transformer) dont le but est de transformer une ligne de résultat en un type d'objet.

Résultat : moins de code... BEAUCOUP MOINS DE CODE !
Moins de répétitions...
Tu peux ensuite faire des méthodes dans Requete dans le cas par exemple où tu ne veux récupérer qu'un seul résultat (parce que les listes, c'est bien, mais quand on sait qu'on n'aura qu'un seul résultat, autant le prévoir)



Pour ce qui est des erreurs, dis toi que les exceptions sont là pour ça : quand tu as un problème technique, elles remontent automatiquement jusqu'en haut pour faire sortir du programme... Sauf si tu les interceptes (catch(Exception)) pour pouvoir les traiter.
Typiquement, dans la classe Requete, tu ne devrais pas avoir de lien vers la couche de présentation. Ta classe Erreur est un composant graphique qui affiche un message d'erreur. Ce que tu dois faire, c'est le plus haut possible dans ton application, catcher l'exception selon le bon type, et ensuite afficher l'erreur qui ira bien... Quitte à faire une classe, mais autant faire une classe avec des méthodes statiques plutôt que de toujours faire un new. Ou créée une méthode dans ton panel principal qui aura pour but d'afficher les erreurs.


com.mysql.jdbc.MysqlDataTruncation : utiliser ce genre d'erreur pour vérifier un format de date, c'est super moche ! Une exception, comme son nom l'indique, c'est une exception au bon fonctionnement de l'application, c'est un problème non prévisible. Tout ce qui est prévisible doit être géré. Sinon, tu fais ce qu'on appelle une gestion par exception, et crois moi, ce n'est pas une bonne chose (tu risques de perdre des informations ou masquer des problèmes)

C'est ce que j'ai justement voulu faire en créant une classe "PageFacture" et une classe "AfficheFacture" où "AfficheFacture" est chargé de récupérer les données depuis la base de données (en utilisant lui aussi un objet de type "Requete") puis de les mettre dans mon JTable. Ma classe "PageFacture" utilise alors un objet de type "AfficheFacture". Je pense néanmoins que tu parles d'une autre façon de découper mais je ne comprends pas laquelle.

Non, tu as bien compris l'idée. Pense néanmoins à aussi créer ton objet Facture ^^

[Alkhan]

Bonjour,

J'ajouterais sur le sujet de la persistance des données et du système de requete, que des frameworks permettent de le faire sans réinventer soi même la chose !
Je pense naturellement aux implementations de la spécification JPA telque Hibernate et EclipseLink pour ne citer que les deux plus répendu.

[Logan Mauzaize]

Il y a aussi jOOQ et QueryDSL SQL