Snake

**Ceytix** · 02/11/2015, 18h29

Bonjour,

Je vous propose un nouvel élément à utiliser : Snake

Snake réalisé en c++ avec SDL, SDL_ttf et SDL_image

Qu'en pensez-vous ?

**koala01** · 11/11/2015, 18h27

Salut,

A vrai dire, il y a beaucoup à en dire, mais pas grand chose de vraiment "bien" (hormis : le fait de remarquer que tu as quand meme fait de ton mieux).

Les remarques que je vais faire seront sans complaisance, mais j'essayerai de justifier chacune d'elles et qu'elle te permettront de t'améliorer

Commençons par les remarques les moins importantes :
Primo, on a l'habitude de respecter la règles "une déclaration / une instruction == une ligne".
Au lieu d'avoir la ligne SDL_Surface *screen, *indexPicture, *levelChoicePicture, *creditsPicture; (au niveau de Index.h), on préférerait avoir queque chose ressemblant à

Code :

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

1
2
3
4
SDL_Surface *screen;
SDL_Surface *indexPicture;
SDL_Surface *levelChoicePicture;
SDL_Surface *creditsPicture;

Cela revient strictement au même, mais, au moins, on identifie beaucoup plus facilement les différentes variables qui sont déclarées (même si, en l'occurrence, il s'agit de membres d'une classe)

Secundo : on se doute que, si tu invoques la fonction printIndex depuis une variable nommée index dont le type est Index, c'est parce que tu veux afficher quelque chose qui a trait avec... un index.

Dés lors, pourquoi nommer cette fonction printIndex et pas, tout simplement, je sais pas moi : print ou mieux, quelque chose comme execute ou mainLoop

De plus, les seules choses qui ont, effectivement un lien quelconque avec la notion de "print" (d'affichage), ce sont les deux premières et les deux dernières lignes de la fonction, qui sont -- de surcoit -- totalement identiques.

Cela me pose plusieurs problèmes de conscience, parce que

En xP (extrem Programming) on a l'habitude d'utiliser l'interjection DRY (Don't Reapeat Yourself ou, si tu préfères en français ne vous répétez pas). C'est le premier conseil que je voudrais te donner
En dehors des quatre lignes qu je vien de citer, tout le reste a une responsabilité totalement différente : celle de gérer la boucle principale du jeu, ce qui implique :
1. que ta cette fonction ne respecte pas le SRP (Single Responsability Principle ou principe de la responsabilité unique en francais)
2. que ta fonction ment sur son objectif principal (au niveau de son nom), car il semble clair que son objectif principal est de gérer la boucle principale du jeu et non de gérer l'affichate

Je te conseillerais donc de scinder cette fonction en deux :

la première (qui pourrait effectivement s'appeler print ou draw) qui ne ferait que s'occuper de l'affichage (blitSurface et split), si l'on prenait la peine fournir l'élément à utiliser comme premier par argument à la fonction SDL_BlitSurface comme paramètre
la deuxième qui s'appellerait execute, run, play ou mainLoop (tu choisi le terme qui te convient le mieux sur ce coup) qui s'occuperait effectivement de traiter la boucle principale et de provoquer l'affichage "quand c'est utile".

La fonction print pourrait d'ailleurs te respecter un peu plus le DRY car, si on y regarde bien, on retrouve encore ces deux mêmes instructions SDL au début et à la fin de la fonction levelChoice... surprenant, non

Ensuite, ta classe Index me fait beaucoup plus penser à une casse gérant le jeu qu'à une classe d'index... Etant donné que le nom que tu donnes aux différents éléments que tu crées / manipule influe énormément sur la manière dont tu les manipules, j'aurais vraiment tendance à renommer cette classe ... Game (par exemple), ce qui justifierait parfaitement d'avoir la fonction execute, run, play ou mainLoop dont je viens de parler.

Tant que nous en sommes à notre classe Index, je n'arrive pas vraiment à comprendre la raison qui t'a poussé à utiliser un pointeur pour son membre game:

Il n'y a aucun héritage en jeu, ce qui fait que tu n'a absolument aucun besoin de polymorphisme
tu le crée dans le constructeur, tu le détruit dans le destructeur, et tu n'y touche plus entre les deux

Autrement dit, tu n'a aucune raison d'en faire un pointeur: un membre "normal" (sous la forme de Snake game;, histoire de garder tes propres termes pour l'instant) aurait parfaitement fait l'affaire! En plus, cela t'aurait évité d'avoir à le créer (à coup de new) et à le détruire (à coup de delete) explicitement.

En outre, c'est généralement une très mauvaise idée de s'emm...der à gérer la mémoire dynamique à la main en C++. On préfère très largement laisser ce soin à ce que l'on appelle des "capsules RAIIsantes", comme les classe std::unique_ptr (nécessite C++11) ou boost::unique_ptr (si tu ne peux vraiment pas utiliser un compilateur "correct") dont le seul objectif sera de libérer les ressources lorsqu'elles ne seront plus nécessaires.

Cela permet d'éviter bien des soucis si, pour une raison ou une autre, ton programme venait à lancer une exception: cela te permet d'avoir un programme beaucoup plus robuste ne souffrant d'aucune fuite mémoire, et, crois moi, c'est un gain des plus intéressants

Pour en terminer avec la classe Index et ses fichiers, il faut savoir QUE L'ON N'INCLUT JAMAIS UN FICHIER .cpp DANS LE CODE. Ce sont les fichier d'en-tête (*.h/ *.hpp) que l'on inclut (note qu'on retrouve la même erreur dans main.cpp

)... Attention, corriger cette erreur t'obligera à corriger ton Makefile

Ensuite, il y a ta classe Snake. La première question que je voudrais te poser est : quelle est la responsabilité de cette classe

Car, si on y regarde bien, le SRP est de nouveau très fort mis à mal :

les fonctions newGame() play(int) font penser à la notion de "partie"
les fontions print() et refreshScore fait penser à la notion de "vue"
les fonctions loadRecord et saveRecord font penser à la notion de sérialisation"
les fonctions placeWalls, placeFood, eventManagment et changeDelays font penser à la notion de "mécanique de jeu" et, pour finir
les fonctions hints et move font penser à la notion du snake en lui-meme

Autrement dit, ta classe Snake regroupe les responsabilités de... cinq notions clairement distinctes. Cela lui fait ... quatre responsabilités de trop

En outre, les deux classes que tu as créées sont symptomatiques de plusieurs autres principes baffoués. En effet, si on y regarde d'un peu plus près, on se rend compte que les deux classes s'occupent

du chargement de différentes ressources (images à utiliser, police de caractères, niveaux, tableaux des records, ...)
de l'utilisation de la SDL, de manière générale,
de la gestion des entrées et des sorties (de manière générale)

Le tout, en plus de gérer un aspect bien particulier que l'on peut effectivement attendre de leur part (tout ce qui a trait au lancement d'une partie pour Index, en attendant qu'elle change de nom et tout ce qui a trait au serpent pour la classe Snake)

En plus de bafouer le SRP (car cela correspond à autant de responsabilités supplémentaires), on se rend compte, entre autres, que tu n'applique absolument pas le DIP (Dependancies Inversion Principle ou principe d'inversion des dépendances), pour ne citer que lui.

En effet, tu serais sans doute bien inspiré de créer un "contexte" qui regroupe servirait de "façade" pour toutes les fonctionnalités propres à la SDL que tu compte utiliser.

Cela t'éviterait très certainement d'avoir à "disperser" tous tes appels aux différentes fonctionnalités de la SDL dans ton code, le rendant à la fois plus sur, plus facile à débugger et plus facile à maintenir

Pour terminer, je me suis un tout petit peu intéressé au code de ta fonction placeWalls, et je me suis rendu compte que, finalement, la seule chose qui changeait dans les différents case que tu as écrit, c'était... le nom du fichier à ouvrir.

Sais tu que la gestion des chaines de caractères (std::string) est le domaine qui propose sans doute le plus de fonctionnalités diverses et variées

Par exemple, C++11 est arrivé avec une flopée de fonctions appelées std::to_string, qui permettent de convertir n'importe quel type primitif en chaine de caractères. L'une d'elle nous permettrait donc de convertir un entier (comme le membre level) en une chaine de caractères équivalente, sous une forme qui pourrait être proche de

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
auto str = std::to_string(level); /* en fonction de la valeur de level, str vaudrait
                                    * "0"
                                    * "1"
                                    * "2"
                                    * "3"
                                    * "4"
                                    * "5"
                                    * "6"
                                    * "7"
                                    * "8"
                                    * ou "9"
                                    */

Une autre fonctionnalité de la classe std::string est la fonction append qui permet de rajouter du texte à la fin de la chaine, sous une forme proche de

Code :

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

1
2
3
 
std::string str{"salut"}; // str = "salut"
str.append(" le monde"); // str = "salut le monde"

En combinant ces deux fonctionnalités, on pourrait parfaitement remplacer une bonne partie de ton code par quelque chose ressemblant à

Code :

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

1
2
3
 
std::string filename{"files/levels/"}; // la partie commune à tous les fichiers que nous pourrons rechercher
filename.append(std::to_string(level)); // le numero du niveau

En outre, il est de règles, en C++, de déclarer les variables "le plus tard possible" ou, si tu préfères, "le plus près possible" de l'endroit où elles seront effectivement utilisées.

Ce qui est chouette, c'est que la classe std::ifstream dispose justement d'un constructeur qui s'attend à recevoir un nom de ficher à ouvrir, ce qui fait que nous pourrions remplacer une bonne partie du code de ta fonction placeWalls par quelque chose qui ressemblerait à

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
33
34
35
36
37
38
39
40
41
 
void Snake::placeWalls(){
    /* j'ai remarqué que la plupart des niveaux plaçaient le serpent à ces positions...
     * considérons cela comme les positions "par défaut"...
     */
     int x{25};
     int y{25};
     /* gérons les niveaux pour lesquels ce n'est pas le cas...
      * question : ne pourrait-on pas envisager d'indiquer cette position directement dans le fichier du niveau?
      * ce serait beaucoup plus évolutif ;)
      */
    if(level==3 || level==6){
          x = 10;
    }else if (level == 5){
        y = 10;
    }else if (level == 8){
        x = 30;
        y = 30;
    } else if (level == 9){
        x = 1;
        y = 1;
    }
    /* le nom du fichier qu'on va manipuler */
    std::string filename{"files/levels/"}; // la partie commune 
    filename.append(std::to_string(level));// la partie spécifique 
    std::ifstream(filname);
    /* si on décidait de placer la position de départ dans le fichier, on la lirait sans doute ici
      * puis, on l'ajouterait aux données qui s'en inquiètent
      */
    xPos.push_front(x);
    yPos.push_front(y);
    /* et enfin, on lit le niveau en lui-même */
    for(int i=0; i<50; i++)
    {
        for(int j=0; j<50; j++)
    	{
            map[j][i] = level_file.get() - '0'; // convert '0' to 0; '1' to 1; etc
    	}
        level_file.get();
    }
}

Avoue que, surtout si tu prend mes proposition en compte, cela te facilite vachement la tâche non

A vrai dire, il y a encore quelques remarques qui nécessiteraient d'être faites sur ton code. Mais je me dis que tu en as déjà assez et que, si tu es motivé, tu en as déjà pour suffisamment de temps pour arriver à toutes les prendre en compte. Je vais donc m'arrêter là

Je voudrais juste préciser une chose : ces remarques n'ont absolument pas pour objectif de te décourager ou de te forcer à abandonner. Bien au contraire: ces remarques ont pour but de t'inciter à évoluer et à t'intéresser à certains aspects auxquels tu n'as, de toute évidence, pas encore porté d'attention.

Tu ne dois donc pas hésiter à poser un maximum de questions sur les remarques que j'ai pu faire, car je me rend compte qu'aucune n'apporte de solution. D'un autre coté, tu devrais pouvoir trouver les solutions par toi-même si on t'oriente un tout petit peu, et je crois que ce sera le meilleur service à te rendre que de te permettre de les chercher par toi même... Qu'en penses-tu

**Ceytix** · 15/11/2015, 17h16

Merci beaucoup pour tous ces conseils,
J'en tiendrai compte la prochaine fois

Snake

Téléchargez C++

Discussions similaires

Partager

Partager