Salut tout le monde
Je viens de coder un petit lecteur audio en C avec SDL, et j'aimerais savoir ce que vous en pensez
Je l'ai fait juste pour m'entraîner, donc si vous voyez des moyens de l'améliorer, n'hésitez pas

Voilà une screen shoot :



Et voici le lien pour télecharger le Lecteur, le Code Source et les Ressources

Si vous trouvez des bugs, n'hésitez pas à me les annoncer

Je compte nettement plus de 50 variables déclarées rien que pour ta fonction main ! C'est complètement énorme (j'ai pas lu plus loin). Il faudrait essayer de repenser le programme en délégant une partie du boulot à des fonctions annexes, utiliser des strucures, et surtout, des tableaux.

Avec un tableau et une énumération, tu peux regrouper par exemple toutes les surfaces qui composent ton interface. Après, pour le blitt, une simple boucle te permet de toutes les blitter. Quand tu auras corrigé ça, ça ira déjà bien mieux je pense.

Tu veux dire qu'il faut regrouper toutes les surfaces du programme dans un tableau !!
Ouais, c'est une bonne idée
Mais quelle est la différence, c'est au niveau de la lisibilité du code ou au niveau technique !!
En tous cas, je vais faire comme tu m'as dis, merci

Tu devrais tester que l'ouverture des ressources (comme les images) et les initialsation des bibliothèque ont bien fonctionné, car dans le cas contraire, ton programme plante surement.

Il y a quelques soucis d'indentations, mais rien de bien méchant.

Tu devrais aérer un peu le switch de ta boucle principale qui n'est pas très lisible.

Citation : Caribou87

Tu devrais tester que l'ouverture des ressources (comme les images) et les initialsation des bibliothèque ont bien fonctionné, car dans le cas contraire, ton programme plante surement.

Merci, mais je pense que toutes les ressources sont livrées avec le programme !! N'est ce pas

Citation : Caribou87

Il y a quelques soucis d'indentations, mais rien de bien méchant.

Peut être parce que je fais l'identation avec deux caractères de décalage

Citation : HighTam

Merci, mais je pense que toutes les ressources sont livrées avec le programme !! N'est ce pas

Oui, mais on ne sait jamais ce qui peut arriver. L'ouverture peut échouer tout de même (problème de droits d'accès au fichier, etc). Donc c'est cool de le tester, ça permet de « crasher proprement ».

Sinon, mon conseil de division comptait surtout pour la lisibilité du code : tu vires une vingtaine de variables et tu fais un tableau, ça te permet aussi de virer 20 opérations en faisant une simple boucle. Tu gagnes en concision et en lisibilité. C'est toujours mieux. Après, techniquement parlant, est-ce que ça change autre chose, j'ai pas d'idée.

EDIT : Foirage zCode.

Bon, voyons : suuposons que je vais déclarer un tableau qui comporte 20 surfaces :

SDL_Surface *surfaces[20] = {NULL};

Donc, toutes les surfaces vont avoir le même nom : surfaces[0], surfaces[1], surfaces[2]...
Alors je serai un peu perdu si je veux modifier une surface (car je ne connais pas exactement son nom) !!
Au contraire :

SDL_Surface *ecran = NULL, *play = NULL, *stop = NULL...

En faisant ça, je peux donner un nom à chaque surface donc je peux facilement l'envoyer à une fonction etc...

C'est juste ce que j'ai dit ou non

C'est juste, mais il y a un moyen de le contourner : les énumérations.

EDIT : Il manquait une étoile, en effet.

enum
{
    PLAY = 0,
    STOP,
    TOTAL
};

{
    SDL_Surface** surfaces = malloc(sizeof *surfaces * TOTAL);
    if(surfaces != NULL)
    {
        surface[PLAY] = ...
        surface[STOP] = ...
    }
}

Présente l'avantage de pouvoir très facilement ajouter des surfaces (il suffit d'intercaler le nom avant TOTAL), de pouvoir boucler pour manipuler toutes les surfaces, etc. Et une liste verticale comme dans le enum fait plus lisible qu'une liste en ligne, séparée par des virgules.

C'est surtout que ça paraît vraiment énorme d'avoir une fonction avec 50 variables. Si les tableaux ne te plaisent pas, coupe ta fonction en morceaux plus petits, ou fais une strucutre.

Citation : GuilOooo

C'est juste, mais il y a un moyen de le contourner : les énumérations.

J'avais complétement oublié les énumérations Merci, c'est la bonne méthode

Hey ! Mais il y a une erreur lors du lancement de ton programme, voilà ce qui s'affiche :



J'ai même essayé de le lancer depuis Program Files, mais ça marche pas aussi .

Citation : MadaraUchiwa

Hey ! Mais il y a une erreur lors du lancement de ton programme, voilà ce qui s'affiche :

C'est bizarre, le programme marche correctement chez moi !!
Bon, essaye de l'éxecuter une autre fois !!

J'ai besoin d'autres avis, si ce problème se répète chez d'autres personnes il faut que je trouve une solution

Citation : HighTam

essaye de l'éxecuter une autre fois !!

Oui, j'ai essayé plusieurs fois et ça marche pas, même si je redémarre mon PC.

Et les autres, ça marche chez vous ?

Je suis pas sous Windows, donc j'aurai du recompiler le code source pour essayer, ce que je n'ai pas eu le temps de faire. Mais il faudrait mettre une copie des ressources en dehors de l'installeur, avec un zip.

Par exemple, on peut faire d'un côté l'installeur, de l'autre un zip avec sources + ressources (pour les autres plates-formes). Sinon, HighTam, tu compiles avec quoi ? Visual C++ ?

Oui, je compile avec Visual C++ !!
Sinon, j'ai donné le main.c avec le ".exe" de l'installation !!

Citation : GuilOooo

C'est juste, mais il y a un moyen de le contourner : les énumérations.

enum
{
    PLAY = 0,
    STOP,
    TOTAL
};

{
    SDL_Surface* surfaces = malloc(sizeof *surfaces * TOTAL);
    if(surfaces != NULL)
    {
        surface[PLAY] = ...
        surface[STOP] = ...
    }
}

Présente l'avantage de pouvoir très facilement ajouter des surfaces (il suffit d'intercaler le nom avant TOTAL), de pouvoir boucler pour manipuler toutes les surfaces, etc. Et une liste verticale comme dans le enum fait plus lisible qu'une liste en ligne, séparée par des virgules.

C'est surtout que ça paraît vraiment énorme d'avoir une fonction avec 50 variables. Si les tableaux ne te plaisent pas, coupe ta fonction en morceaux plus petits, ou fais une strucutre.

Ton exemple marche pas chez moi GuilOooo.
Par contre ça oui :

enum{
    PLAY = 0,
    STOP,
    TOTAL
};
SDL_Surface *surfaces[TOTAL];

surface[PLAY] = ...
surface[STOP] = ...

Citation : 21

Ton exemple marche pas chez moi GuilOooo.

Il a du oublier une étoile :

SDL_Surface** surfaces = malloc(sizeof *surfaces * TOTAL);

Citation : 21

Par contre ça oui : [...]

Je préfère aussi cette solution, une énumération étant une constante entière, ça ne pose pas de problème.

Citation : GuilOooo

Par exemple, on peut faire d'un côté l'installeur, de l'autre un zip avec sources + ressources (pour les autres plates-formes).

Voilà, maintenant, l'installation les sources et les ressources sont disponibles ici (version mise à jour)

Citation : MadaraUchiwa

Oui, j'ai essayé plusieurs fois et ça marche pas, même si je redémarre mon PC.

Et les autres, ça marche chez vous ?

SVP, j'ai besoin de savoir si ça marche chez les autres !! Parce que (je crois) il n'y a pas de fautes dans le code !!
Et si vous trouvez d'autres bugs, n'hésitez pas de me les annoncer Merci

l'exe après installations ne fonctionne pas.
Peut être il manque une dll ou chepakoi...

J'ai essayé de télecharger le ".rar" et j'ai installé le lecteur et ça marche correctement
Je ne sais pas ce qui se passe !!

Citation : 21

l'exe après installations ne fonctionne pas.

Il t'affiche une erreur ??

J'ai exactement la même erreur que MadaraUchiwa.

Tu dois oublier un fichier je pense.

J'ai téléchargé le nouveau zip mais toujours la même erreur.

Salut.

Lorsque tu déclare un pointeur, cela n'est pas obliger de mettre NULL, car dans le cas d'un pointeur, lors de son initialisation il est automatiquement mis à NULL.

Ensuite, utilise plutot la librairie FMOD Ex. qui est plus performante et qui elle est a jour (et fonctionne sur les arch linux64)

Puis question portabilité : n'utilise pas l'API Windows.

Et en effet test l'ouverture et l'initialisation des tes pointeurs et tes images.


Autre chose, ici au niveau de la lisibilité :

if (event.button.x >= positionPlay.x && event.button.x <= (positionPlay.x + play->w) && event.button.y >= positionPlay.y && event.button.y <= (positionPlay.y + play->h))

{

A remplacer par :

if (event.button.x >= positionPlay.x
    && event.button.x <= (positionPlay.x + play->w)
    && event.button.y >= positionPlay.y
    && event.button.y <= (positionPlay.y + play->h))
{

Pour tes variables, la solution enum+tableau est parfaite.

edit : changement d'approche.

Citation : Link/DD

Salut.Lorsque tu déclare un pointeur, cela n'est pas obliger de mettre NULL, car dans le cas d'un pointeur, lors de son initialisation il est automatiquement mis à NULL.

Comment expliques-tu alors ceci :

#include <stdio.h>

void check(int* p)
{
  printf("p à NULL ? ");
  if(!p)
    printf("Oui\n");
  else
    printf("Non\n");
}

int main(void)
{
  int *p1,*p2 = NULL;

  check(p1);
  check(p2);

  return 0;
}

p à NULL ? Non
p à NULL ? Oui

Citation : Link/DD

Et en effet test l'ouverture et l'initialisation des tes pointeurs et tes images.

Citation : Caribou87

Tu devrais tester que l'ouverture des ressources (comme les images) et les initialsation des bibliothèque ont bien fonctionné

C'est fait Merci

Citation : Link/DD

Autre chose, ici au niveau de la lisibilité :

if (event.button.x >= positionPlay.x && event.button.x <= (positionPlay.x + play->w) && event.button.y >= positionPlay.y && event.button.y <= (positionPlay.y + play->h))

{

A remplacer par :

if (event.button.x >= positionPlay.x
    && event.button.x <= (positionPlay.x + play->w)
    && event.button.y >= positionPlay.y
    && event.button.y <= (positionPlay.y + play->h))
{

C'est fait aussi

Citation : Link/DD

Puis question portabilité : n'utilise pas l'API Windows.

Qu'est ce que je peux utiliser alors ??

Citation : Link/DD

Pour tes variables, la solution enum+tableau est parfaite.

Citation : GuilOooo

C'est juste, mais il y a un moyen de le contourner : les énumérations.

Je suis entrain de travailler pour les régler Merci

Citation : Link/DD

Lorsque tu déclare un pointeur, cela n'est pas obliger de mettre NULL, car dans le cas d'un pointeur, lors de son initialisation il est automatiquement mis à NULL.

Je ne pense pas !!
D'ailleurs la démonstration de freecircus nous montre ça !!

freecircus: Autant pour moi alors

Citation : 21

J'ai exactement la même erreur que MadaraUchiwa.

Tu dois oublier un fichier je pense.

Citation : MadaraUchiwa

J'ai téléchargé le nouveau zip mais toujours la même erreur.

C'est bizarre Le programme fonctionne chez moi

Citation : 21

Tu dois oublier un fichier je pense.

Non, parce que quand je télecharge et j'installe le programme, ça fonctionne
Mais, qu'est ce qui se passe

Tu compiles avec Visual C++. Par défaut, Visual C++ inclut dans ton exécutable tout un tas d'informations qui font qu'il est beaucoup plus facile de retrouver les bugs au sein de ton programme. En contrepatie, la configuration par défaut fait que ton programme dépend de Visual C++ pour être lancé. Comme il est installé chez toi, pas de problèmes. Par contre, il n'en va pas de même chez les autres.

Essaie de changer la cible « Debug » en « Release », et/ou cherches sur ce forum même des problèmes de ce style avec Visual C++ : la réponse a déjà été donnée plusieurs fois.

Sinon, il manquait bien une étoile dans mon code. Mais c'était pour illustrer l'idée. Au passage, je ne sais pas si le fait d'utiliser une constante type énumération pour initialiser un tableau statique en fait un VLA ou pas. Dans le doute, j'ai préféré le malloc.

Citation : GuilOooo

Au passage, je ne sais pas si le fait d'utiliser une constante type énumération pour initialiser un tableau statique en fait un VLA ou pas. Dans le doute, j'ai préféré le malloc.

Si je ne dis pas de bêtise, le contenu des enum est traité comme une liste de define, donc pas de VLA.

J'ai cherché vite fait dans la norme, j'ai trouvé ça (dernier draft, Chapitre 6.6 - Constant expressions, point 6) :

Citation

An integer constant expression shall have integer type and shall only have operands that are integer constants, enumeration constants, ...

Donc, en gros, une expression de type « entier constant » ne contiendra que des constantes entières et des constantes provenant d'énumérations (entre autres choses). Les constantes et les valeurs énum sont donc « dans le même panier ». Par contre, je n'arrive pas à comprendre s'il faut ici comprendre constante comme « constante littérale » ou comme « variable de type const [...] ». Et je n'ai pas le temps de chercher à la page sur la déclaration de tableaux.

Si tu veux être fixé, essaie de compiler le code en question en réglant gcc sur le standard C89 strict. Il te mettra un warning (une erreur ?) si tu essaies de faire un VLA.

Citation : GuilOooo

Au passage, je ne sais pas si le fait d'utiliser une constante type énumération pour initialiser un tableau statique en fait un VLA ou pas. Dans le doute, j'ai préféré le malloc.

C'est ce que j'avais compris en lisant ton code, quand j'ai dis : "une énumération étant une constante entière, ça ne pose pas de problème.", par "pas de problème" je sous-entendais pas de VLA
Pour être clair :

/* ne compile pas en C89 */

const int i = 1;
char tab[i];

/* compile */

enum { ELEMT = 1 };
char tab[ELEMT];