Java/FX: audit de code

Salut les zestes !

Ça fait maintenant pas mal de temps que je code, sur divers projets à droite, à gauche, bref, un peu partout ; récemment, je me suis dit quelque chose: C’est bien beau la console, et je l’utilise beaucoup, mais j’aimerai un système un peu plus… "propre", a.k.a. GUI (IHM pour les français), bref une joulie fenêtre toute belle et plein de couleurs brillantes !

Plus sérieusement, j’ai donc commencé à chercher quelques docs, quelques tutoriels, quelques sources, un point de départ, et après avoir essayé plusieurs solutions (dont l’"obsolète" Swing/AWT, Qt, GTK+ ou encore Tkinter, oui oui j’ai fait les tests sur plusieurs langages !), j’ai fini par retenir JavaFX, qui semble le plus approprié à mes besoins, mais aussi le plus abordable.

Dans le même temps, j’ai commencé à aider un ami en galère en Java, et lui ai proposé de commencer la réalisation d’un programme "To-do", un bloc notes sous la forme d’une liste de notes.
Je me suis alors dit: "Mais pourquoi pas le faire, moi aussi ?".

J’ai donc commencé par coder une version "console" de l’application, que j’ai réalisé sans difficultés particulières, puis j’ai continué à bricoler avec JavaFX (en faisant mes premiers pas avec les fichiers FXML), cette fois ci dans le contexte d’un programme concret, et plus de simples tests.

Je suis aujourd’hui devant une version qui me semble fonctionnelle, et je me tourne donc vers vous pour collecter des avis objectifs sur mon code, en vous demandant de m’aider à l’améliorer, et dans le même temps, m’améliorer en développement d’interfaces graphiques !

Merci d’avoir lu ce petit pavé, et merci d’avance pour toute aide/contribution/conseil que vous m’apporteriez !

Code

Afin de vous permettre de consulter librement le code, je l’ai hébergé sur Gitlab (il me servira aussi à garder note des améliorations/modifications que j’apporterai alors), à cette addresse. On notera l’absence d’un README.md, que j’ai jugé inutile, vu que tout a été expliqué ici.

De plus, si vous voulez directement télécharger une version packagée du programme afin de tester, il vous suffit de télécharger et décompresser ce fichier (note_jar.zip, 5.9Mo).

Bonjour,

C’est toujours une bonne idée pour apprendre de pratiquer et s’évaluer à la fin de son travail. Étant donné que tu demandes surtout des retours sur la partie IHM, je me suis concentré sur ce morceau.

Le binaire

Ton binaire a un souci, lorsqu’on le lance, par défaut il lance la version en console. Pour le remettre d’aplomb il faut que tu remplace dans ton fichier META-INF/MANIFEST.MF (contenu dans note.jar) Main-Class: Main par Main-Class: ui.GUIApplication

L’ihm de l’application

C’est simple et efficace, ça fait le job demandé. Je vois pas mal d’évolutions possibles (éditer une note, rajouter une description, une date, pouvoir checker une note, etc.) qui te permettront de manipuler plus d’une fenêtre.

Le code

Les dépendances

C’est un peu hors sujet, mais c’est une mauvaise pratique de versionner des binaires dans ton code (tous tes fichiers jar). Tes dépendances (sqlite-jdbc-3.16.1.jar par exemple) devrait être gérée avec un outil fait pour ça (maven ou gradle)

Le style

Je ne l’ai vu qu’une seule fois (mais en même temps tu n’as qu’une vue), mais tout comme on évite de mettre le code css directement dans une page html, on évitera aussi de mettre du css directement dans un code fxml (comme c’est le cas ici). La bonne pratique est d’avoir une feuille de style en dehors qui s’en occupe. Mais dans ton cas, ton application n’est pas énorme.

La vue

Tu m’as l’air d’avoir compris le fonctionnement des layout FXML, c’est une bonne chose.

Le contrôleur

• Ton style de code n’est pas logique, tu mélange snake_case (status_printer, notes_list, note_rm_btn, note_add_field) et camelCase. En Java on est habituellement sur du camelCase
• Normalement, plutôt que d’implémenter Initializable tu devrais utiliser l’annotation @FXML pour les injections.
• Ta méthode refreshList() ne devrait pas exister, le rafraichissement devrait être implicite. Tu dois te contenter de gérer une ObservableList<String> en tant qu’attribut de ta classe. Cette liste est initialisée au début avec la liste des valeurs, lorsque tu supprime un éléments de cette liste, ta vue est automatiquement mise à jour puisque le composant JavaFx est fait ainsi. Je te conseille de jeter un œil à la documentation sur ce composant. ça t’évitera un rafraichissement qui consiste finalement à tout effacer et réécrire le contenu.
• Le système de binding JavaFX te permet d’éviter de multiplier les déclarations comme celles-ci. Tu peux tout simplement passer par quelque chose comme note_rm_btn.disableProperty().bind(notes_list.getSelectionModel().selectedItemProperty().isNull()); dans la méthode initialize, et le button s’adaptera tout seul après. Cette instruction permet de dire au bouton "désactive toi à chaque fois qu’aucun élément de la liste n’est sélectionné"

Normalement ces petites choses devraient rendre ton contrôleur plus pro.

Voilà, pour une premier retour qui j’espère t’aidera à t’améliorer.

Première correction, encore beaucoup de boulot, mais je me retrouve… "bloqué".

Bref, réponse apportée aux premiers retours:

• Avant tout, merci !
• Le binaire: C’était un choix de faire en sorte que le contrôleur de Main lance par défaut la version console, et mette à disposition un argument pour lancer l’interface graphique. Choix maladroit, maintenant inversé.
• L’ihm de l’application: En effet, c’est très limité, et j’ai pensé à ces quelques améliorations, mais aussi à un système de tags, mais je préférais travailler étapes par étapes et déjà réussir à avoir une logique de développement correcte avant de me retrouver avec trop de code a corriger
• Les dépendances: En effet, une erreur que j’aurai généralement évitée.
  Le seul correctif que j’ai effectué ici par contre a été de renommer l’archive en sqlite_jdbc.jar.
  J’ai tenté de faire un script Maven, mais je galère encore beaucoup avec cette tech, et j’ai décidé de laisser ça de côté et me concentrer sur la partie "application" plutôt que gestion de dépendances/cycles de développement. Je m’y mettrai plus tard.
• Le style: en effet, c’est une amélioration que je compte faire, une fois fini de fixer chaque "problème" au niveau du code.
• Le contrôleur: vu que j’ai pas mal à répondre, je sépare ça en une autre partie.

Le contrôleur

Alors, pas mal de problèmes de ce côté en effet, surtout avec la partie refreshList, très.. "hacky", que j’ai fait un peu comme première solution pour répondre à ce problème de rafraichissement.

• Style de nommage corrigé.
• J’ai changé la portée des champs (> private), et ajouté les annotations @FXML devant.
  Par contre, j’ai conservé le implements Initializable et la fonction initialize, car je n’ai pas trouvé d’autres moyens d’initialiser l’interface et y bind tous les évènements.
• Celui là est … toujours très problématique.
  Je n’ai pas tellement compris le fonctionnement de ObservableList, mais j’ai commencé à essayer d’implémenter une version observable de la liste de textes (encore une fois, très "hacky", temporaire, le temps que je comprenne comment m’en servir).
  Ma principale problématique est: comment utiliser l’OL avec des objets custom (Note), et faire en sorte de n’afficher que le texte (en tentant d’éviter la redéfinition de toString() ). Bref, je ne sais pas comment fonctionne ce container, j’ai regardé la ObservableListBase, et ai trouvé quelques classes qui pourraient éventuellement remplir le job (par exemple TransformationListBase) mais sans grand succès. J’ai un peu cherché quelques tutoriels, mais les seuls que j’ai pu trouver étaient soit obsolètes soit assez mal présentés, et je n’ai pas réussi à comprendre. Quelques éclaircissements ? (Voir commit "intermédiaire" fa886d18)
• En effet, je ne savais pas, je note.
  Je regarderai de plus près la doc sur les binds la prochaine fois

Bref, encore 1 élément à terminer, et quelques blocs à éventuellement ajouter.

Autre question

Maintenant, je me pose une autre question, que je me suis, en fait, toujours posé lors de la conception de GUIs:

Comment, lorsqu’on a plusieurs vues et qu’on veut changer d’une vue à une autre, changer ?
En gardant en tête que l’action de changement est lancée avec un bouton membre d’une vue, mais du coup, si l’action de chargement est effectuée dans le contrôleur d’une des vues, pour charger une autre vue, ça me semble très… "moche" (aka passer l’instance de Stage (primaryStage) à chaque contrôleur, pour qu’il puisse manuellement changer la Scene).
Y a-t-il une sorte de "routeur" pour permettre de gérer ça depuis l’application (GUIApplication) ou depuis une classe Service dédiée à la gestion de vues ?

J’ai tenté de faire un script Maven, mais je galère encore beaucoup avec cette tech, et j’ai décidé de laisser ça de côté et me concentrer sur la partie "application" plutôt que gestion de dépendances/cycles de développement. Je m’y mettrai plus tard.

Comme je l’ai dis, si ton but est de bosser l’IHM, oui ça peut être traité plus tard.

Par contre, j’ai conservé le implements Initializable et la fonction initialize, car je n’ai pas trouvé d’autres moyens d’initialiser l’interface et y bind tous les évènements.

En fait tu dois aussi annoter les méthodes dans lesquelles tu souhaiterai utiliser tes attributs. Regarde par exemple cette méthode initialize

Je n’ai pas tellement compris le fonctionnement de ObservableList,

En fait avec cette doc devrait t’aider je pense. La partie qui t’interesse est ça :

1
2
3
4

ListView<String> list = new ListView<String>();
ObservableList<String> items = FXCollections.observableArrayList (
    "Single", "Double", "Suite", "Family App");
list.setItems(items);

La ligne 2 te permet d’initialiser une liste observable et la ligne 3 mappe ta vue sur la liste observable. Donc dans ton programme tu te contente de manipuler la liste observable et ton ihm se rafraichira toute seule.

Comment, lorsqu’on a plusieurs vues et qu’on veut changer d’une vue à une autre, changer ? […]

La façon la plus simple de faire ce que tu décris, est de passer dans ton contrôleur une instance de ta classe GUIApplication et d’avoir dans ta classe GUIApplication une méthode qui permet de charger n’importe quel vue.

En gros quelque chose du genre (je surligne ce que je rajoute par rapport a ton code) :

 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
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56

public class GUIApplication extends Application {
    private static final String title = "Notes";
    private Stage primaryStage;

    public static void guimain(String[] args) {
        launch(args);
    }

    /**
     * Starts the main GUI application
     * @param ps primary stage
     * @throws Exception in case something's wrong
     */
    @Override
    public void start(Stage ps) throws Exception {
        this.primaryStage = ps;
        this.primaryStage.setTitle(title);
        this.primaryStage.setResizable(false);
        loadView("List")
    }

    public void loadView(String fxmlNameFile) {
        Parent view = FXMLLoader.load(getClass().getResource("views/"+fxmlNameFile+".fxml"));
        if (view.getController() instance of CustomController) {
            ((CustomController) view.getController()).setGUIApplication(this);
        }
        this.primaryStage.setScene(new Scene(view , 800, 600));
        this.primaryStage.show();
    }
}

// interface qui définit la méthode qui permettra a certains contrôleurs d'être chargés 
public interface CustomController {
    public void setGUIApplication(GUIApplication application);
}

public class ListController implements CustomController {
    @FXML private Label statusPrinter;
    @FXML private ListView<String> notesList;
    @FXML private Button noteRmBtn;
    @FXML private TextField noteAddField;
    GUIApplication application; // tu remarqueras qu'on a pas besoin de @fxml ici


    private NoteHandler nh;

    public void setGUIApplication(GUIApplication application) {
        this.application = application
    }

    // le reste de ton code

    public goToOtherView() {
        this.application.loadView("other") // il chargera la vue présente dans views/other.fxml
    }
}

Pour le moment c’est la seule mécanique qui permette de gérer les chargements de plusieurs vues.

Voilà, en espérant que ça t’aide.