[c] Comment ameliorer mon code
Résolu/Fermé
lirycs78
Messages postés
103
Date d'inscription
vendredi 7 juillet 2006
Statut
Membre
Dernière intervention
7 janvier 2010
-
21 mars 2009 à 15:49
fiddy Messages postés 11069 Date d'inscription samedi 5 mai 2007 Statut Contributeur Dernière intervention 23 avril 2022 - 22 mars 2009 à 22:16
fiddy Messages postés 11069 Date d'inscription samedi 5 mai 2007 Statut Contributeur Dernière intervention 23 avril 2022 - 22 mars 2009 à 22:16
A voir également:
- [c] Comment ameliorer mon code
- Code asci - Guide
- Code puk bloqué - Guide
- Code telephone oublié - Guide
- Code activation windows 10 - Guide
- Code gta 4 ps4 - Guide
13 réponses
fiddy
Messages postés
11069
Date d'inscription
samedi 5 mai 2007
Statut
Contributeur
Dernière intervention
23 avril 2022
1 844
22 mars 2009 à 22:12
22 mars 2009 à 22:12
Le coup du flush du buffer clavier doit se faire après le fgets. Pas avant., car tu ne peux pas être sûr de ce que contient le buffer.
Voici un exemple de comment réaliser une entrée robuste, en enlevant le \n, et en vidant le buffer.
Par contre si tu remplaces ton fgets par mon code, tu auras l'impression qu'il ne s'exécute pas. Ceci est normal puisqu'il faut également que tu corriges ton scanf("%d") qui envoie un '\n' dans le buffer. Il faut donc le vider également par un getchar(). Mais le mieux serait de le remplacer par un fgets suivi d'un strtol.
Voici un exemple de comment réaliser une entrée robuste, en enlevant le \n, et en vidant le buffer.
if(fgets(name,sizeof name,stdin)!=NULL) { char *p=strchr(name,'\n'); if(p==NULL) { // si pas de \n dans la chaîne, alors le buffer n'est pas vide char c; while((c=getchar())!='\n' && c!=EOF); } else { //sinon, on le remplace par '\0' *p='\0'; } }
Par contre si tu remplaces ton fgets par mon code, tu auras l'impression qu'il ne s'exécute pas. Ceci est normal puisqu'il faut également que tu corriges ton scanf("%d") qui envoie un '\n' dans le buffer. Il faut donc le vider également par un getchar(). Mais le mieux serait de le remplacer par un fgets suivi d'un strtol.
fiddy
Messages postés
11069
Date d'inscription
samedi 5 mai 2007
Statut
Contributeur
Dernière intervention
23 avril 2022
1 844
21 mars 2009 à 16:26
21 mars 2009 à 16:26
Salut,
Dans un premier temps, il faut le rendre robuste :
Par exemple, lorsque tu fais un scanf("%d",...) qui te dit que l'utilisateur va rentrer un nombre. Pareil pour scanf("%s") l'utilisateur peut rentrer plus de caractères qu'autorisés. Il vaut donc mieux mettre fgets().
Les while(!feof) sont moins bien que les while(fgets(...) != NULL).
Tu peux réduire la portée des variables.
Par exemple :
//char caractere;
if (...) {
char caractere; //est mieux ici. Réduction de la portée
}
L'instruction : fflush(stdin); est à éviter. Son comportement est indéfini.
Pour vider le buffer clavier, il faut mettre : char c; while((c=getchar())!='\n' && c!=EOF);
Attention, fgets stocke le \n seulement lorsqu'il a la place. Donc pour les longues chaînes, le \n ne sera chiffré et pour les chaînes courtes, il le sera. Il vaut donc mieux le virer dans tous les cas (ou le laisser) avec strchr().
Ensuite, tu peux renforcer les prototypes. Par exemple :
void lire_file(char name[]); peut devenir : void lire_file(const char name[]). Cela permet d'éviter des erreurs en tant que codeur, et dans certains cas améliorer l'optimisation de codes apportées par le compilateur. De même pour les autres prototypes (quand c'est possible bien sûr ^^).
int main() doit devenir : int main(void)
Ensuite, pour l'optimisation, ça ne sera pas flagrant puisque l'algorithme n'est pas suffisamment complexe pour être améliorable. Du coup, ce sera juste de l'optimisation de code.
Du genre, remplacer les : printf("%s\n",text) ; par des puts(text);
Cela est légèrement plus rapide. Mais le compilateur sait le faire tout seul.
Par contre à la place de :
for(i = 0; i < strlen(text); i++){
text[i] = text[i] + 5;
}
tu pourrais mettre :
char *p=texte;
while(*texte) *(texte++)+=5;
Ce qui est plus rapide.
Et enfin, cela manque de remarques ,même si le code n'est pas compliqué ;-).
PS : dans lire_fichier(), tu as oublié de refermer le fichier avec fclose().
Si t'as des questions sur mes remarques, n'hésite pas.
Malgré tous les remarques que j'ai faites, ton code est quand même bien (juste que t'as demandé des critiques, en voici en voilà ;-)).
Cdlt
Dans un premier temps, il faut le rendre robuste :
Par exemple, lorsque tu fais un scanf("%d",...) qui te dit que l'utilisateur va rentrer un nombre. Pareil pour scanf("%s") l'utilisateur peut rentrer plus de caractères qu'autorisés. Il vaut donc mieux mettre fgets().
Les while(!feof) sont moins bien que les while(fgets(...) != NULL).
Tu peux réduire la portée des variables.
Par exemple :
//char caractere;
if (...) {
char caractere; //est mieux ici. Réduction de la portée
}
L'instruction : fflush(stdin); est à éviter. Son comportement est indéfini.
Pour vider le buffer clavier, il faut mettre : char c; while((c=getchar())!='\n' && c!=EOF);
Attention, fgets stocke le \n seulement lorsqu'il a la place. Donc pour les longues chaînes, le \n ne sera chiffré et pour les chaînes courtes, il le sera. Il vaut donc mieux le virer dans tous les cas (ou le laisser) avec strchr().
Ensuite, tu peux renforcer les prototypes. Par exemple :
void lire_file(char name[]); peut devenir : void lire_file(const char name[]). Cela permet d'éviter des erreurs en tant que codeur, et dans certains cas améliorer l'optimisation de codes apportées par le compilateur. De même pour les autres prototypes (quand c'est possible bien sûr ^^).
int main() doit devenir : int main(void)
Ensuite, pour l'optimisation, ça ne sera pas flagrant puisque l'algorithme n'est pas suffisamment complexe pour être améliorable. Du coup, ce sera juste de l'optimisation de code.
Du genre, remplacer les : printf("%s\n",text) ; par des puts(text);
Cela est légèrement plus rapide. Mais le compilateur sait le faire tout seul.
Par contre à la place de :
for(i = 0; i < strlen(text); i++){
text[i] = text[i] + 5;
}
tu pourrais mettre :
char *p=texte;
while(*texte) *(texte++)+=5;
Ce qui est plus rapide.
Et enfin, cela manque de remarques ,même si le code n'est pas compliqué ;-).
PS : dans lire_fichier(), tu as oublié de refermer le fichier avec fclose().
Si t'as des questions sur mes remarques, n'hésite pas.
Malgré tous les remarques que j'ai faites, ton code est quand même bien (juste que t'as demandé des critiques, en voici en voilà ;-)).
Cdlt
fiddy
Messages postés
11069
Date d'inscription
samedi 5 mai 2007
Statut
Contributeur
Dernière intervention
23 avril 2022
1 844
21 mars 2009 à 23:42
21 mars 2009 à 23:42
Normal que ça ne marche pas.
J'ai tout dis dans mon premier post. Lorsque tu utilises fgets, il se peut que le '\n' se stocke dans la chaîne lorsque cette dernière est courte. Ceci fait donc foirer ton fopen. Voilà pourquoi je disais qu'il fallait utiliser strchr.
De plus :
if((gc != '\n')||(gc != '\0')){
est à éviter si tu veux faire du bon code. Lorsque tu utilises fgets, tu testes s'il y a \n dans la chaîne, si c'est le cas tu le remplaces par \0 sinon, tu effectues while(getchar()...).
J'ai tout dis dans mon premier post. Lorsque tu utilises fgets, il se peut que le '\n' se stocke dans la chaîne lorsque cette dernière est courte. Ceci fait donc foirer ton fopen. Voilà pourquoi je disais qu'il fallait utiliser strchr.
De plus :
if((gc != '\n')||(gc != '\0')){
est à éviter si tu veux faire du bon code. Lorsque tu utilises fgets, tu testes s'il y a \n dans la chaîne, si c'est le cas tu le remplaces par \0 sinon, tu effectues while(getchar()...).
Enfin un code bien écrit qui donne envie de le lire.
Le première réflexion est: "Tiens, il n'y a pas de commentaires!".
ligne 30: recup_texte(&choix);
Quel est l'intérêt de récupérer la variable choix ? Elle n'est pas utilisée par la suite !
La fonction void recup_texte (int choix) suffit.
ligne 34: scanf("%s",name);
Brrrrrr, ça craint ! Il n'y a pas de contrôle sur le nombre de caractères saisis.
Préférer le fonction fgets().
ligne 47: unsigned char gc = getchar();
ligne 47: if((gc != '\n')||(gc != '\0') ){
ligne 47: fgets(text, sizeof(text), stdin);
ligne 47: fflush(stdin);
ligne 47: }
Je me pose une question: ne manque-t-il pas le premier caractère à la chaîne saisie ?
lignes 87 et 95: FILE *fichier = NULL;
Initialisation totalement inutile et consommatrice d'octets de code.
ligne 94: void lire_file(char name[]){
Dans cette fonction il n'y a pas de fclose(fichier);.
D'autre part, comme pour la fonction ecrire_file (dont le nom, en passant, me fait sourire; pourquoi ne pas écrire: ecrire_fichier ou write_file ? mais je ne fais que sourire, chacun est libre de son choix ;-) ), je n'aime pas beaucoup char name[], je préfère const char *name.
lignes 46 à 65, 99 à 103, 105 à 109: l'indentation n'est pas respectée
Tu as voulu une correction, tu l'as eu ! Alors ne viens pas te plaindre ;-)))
Je n'ai pas tout disséqué, sans doute reste-t-il quelques remarques.
Bon courage.
Le première réflexion est: "Tiens, il n'y a pas de commentaires!".
ligne 30: recup_texte(&choix);
Quel est l'intérêt de récupérer la variable choix ? Elle n'est pas utilisée par la suite !
La fonction void recup_texte (int choix) suffit.
ligne 34: scanf("%s",name);
Brrrrrr, ça craint ! Il n'y a pas de contrôle sur le nombre de caractères saisis.
Préférer le fonction fgets().
ligne 47: unsigned char gc = getchar();
ligne 47: if((gc != '\n')||(gc != '\0') ){
ligne 47: fgets(text, sizeof(text), stdin);
ligne 47: fflush(stdin);
ligne 47: }
Je me pose une question: ne manque-t-il pas le premier caractère à la chaîne saisie ?
lignes 87 et 95: FILE *fichier = NULL;
Initialisation totalement inutile et consommatrice d'octets de code.
ligne 94: void lire_file(char name[]){
Dans cette fonction il n'y a pas de fclose(fichier);.
D'autre part, comme pour la fonction ecrire_file (dont le nom, en passant, me fait sourire; pourquoi ne pas écrire: ecrire_fichier ou write_file ? mais je ne fais que sourire, chacun est libre de son choix ;-) ), je n'aime pas beaucoup char name[], je préfère const char *name.
lignes 46 à 65, 99 à 103, 105 à 109: l'indentation n'est pas respectée
Tu as voulu une correction, tu l'as eu ! Alors ne viens pas te plaindre ;-)))
Je n'ai pas tout disséqué, sans doute reste-t-il quelques remarques.
Bon courage.
Vous n’avez pas trouvé la réponse que vous recherchez ?
Posez votre question
lirycs78
Messages postés
103
Date d'inscription
vendredi 7 juillet 2006
Statut
Membre
Dernière intervention
7 janvier 2010
1
21 mars 2009 à 18:12
21 mars 2009 à 18:12
Merci pour vos reponse :) Je ne me plaind pas bien au contraire c'est avec les critiques qu'on avance :D
Merci de votre aide je vais faire mes corrections
Merci de votre aide je vais faire mes corrections
lirycs78
Messages postés
103
Date d'inscription
vendredi 7 juillet 2006
Statut
Membre
Dernière intervention
7 janvier 2010
1
21 mars 2009 à 18:22
21 mars 2009 à 18:22
switch(choix){
case 1:
puts("Entrez une phrase a crypter");
recup_texte(choix);
break;
case 2:
puts("Entrez le chemin du fichier");
fgets(name, 20,stdin);
lire_file(name);
fflush(stdin);
break;
default:
puts("Erreur !");
break;
}
Le probléme c'est que quand je met fgets a la pace du scanf, quand je compile le fgets ne marche pas :/
Il ne me demande pas de rentrer des caractére :/
case 1:
puts("Entrez une phrase a crypter");
recup_texte(choix);
break;
case 2:
puts("Entrez le chemin du fichier");
fgets(name, 20,stdin);
lire_file(name);
fflush(stdin);
break;
default:
puts("Erreur !");
break;
}
Le probléme c'est que quand je met fgets a la pace du scanf, quand je compile le fgets ne marche pas :/
Il ne me demande pas de rentrer des caractére :/
fiddy
Messages postés
11069
Date d'inscription
samedi 5 mai 2007
Statut
Contributeur
Dernière intervention
23 avril 2022
1 844
21 mars 2009 à 18:32
21 mars 2009 à 18:32
T'as pas dû tenir compte de ma remarque à propos de fflush(stdin).
Le fait que tu aies l'impression que fgets ne marche pas est une conséquence du comportement indéfini de fflush sur stdin. Utilise plutôt le code que je t'ai donné dans mon premier poste à la place de fflush(stdin).
Et là ton fgets devrait marcher.
Le fait que tu aies l'impression que fgets ne marche pas est une conséquence du comportement indéfini de fflush sur stdin. Utilise plutôt le code que je t'ai donné dans mon premier poste à la place de fflush(stdin).
Et là ton fgets devrait marcher.
lirycs78
Messages postés
103
Date d'inscription
vendredi 7 juillet 2006
Statut
Membre
Dernière intervention
7 janvier 2010
1
21 mars 2009 à 18:56
21 mars 2009 à 18:56
switch(choix){
case 1:
puts("Entrez une phrase a crypter");
recup_texte(choix);
break;
case 2:
puts("Entrez le chemin du fichier");
fgets(name, 20,stdin);
lire_file(name);
break;
default:
puts("Erreur !");
break;
}
return 0;
}
J'ai enlever le fflush mais sa ne marche pas :/
Par aileur jai essayer en mettant :
char gc = getchar();
if((gc != '\n') || (gc != '\0')){
fgets(name, 20,stdin);
}
mais sa ne marche pas :s
merci de ton aide
case 1:
puts("Entrez une phrase a crypter");
recup_texte(choix);
break;
case 2:
puts("Entrez le chemin du fichier");
fgets(name, 20,stdin);
lire_file(name);
break;
default:
puts("Erreur !");
break;
}
return 0;
}
J'ai enlever le fflush mais sa ne marche pas :/
Par aileur jai essayer en mettant :
char gc = getchar();
if((gc != '\n') || (gc != '\0')){
fgets(name, 20,stdin);
}
mais sa ne marche pas :s
merci de ton aide
fiddy
Messages postés
11069
Date d'inscription
samedi 5 mai 2007
Statut
Contributeur
Dernière intervention
23 avril 2022
1 844
21 mars 2009 à 18:59
21 mars 2009 à 18:59
Relis bien mon post ^^.
J'ai mis :
Ce qui est vraiment différent de :
char gc = getchar();
if((gc != '\n') || (gc != '\0')){
J'ai mis :
char c; while((c=getchar())!='\n' && c!=EOF);
Ce qui est vraiment différent de :
char gc = getchar();
if((gc != '\n') || (gc != '\0')){
lirycs78
Messages postés
103
Date d'inscription
vendredi 7 juillet 2006
Statut
Membre
Dernière intervention
7 janvier 2010
1
21 mars 2009 à 18:59
21 mars 2009 à 18:59
switch(choix){
case 1:
puts("Entrez une phrase a crypter");
recup_texte(choix);
break;
case 2:
puts("Entrez le chemin du fichier");
char gc = getchar();
if((gc != '\n')||(gc != '\0')){
fgets(name, 20,stdin);
printf("%s",name);
lire_file(name);
}
break;
default:
puts("Erreur !");
break;
}
return 0;
}
Le fgets est bien pris en compte, deplus il affiche bien le nom du fichier que je rentre mais la fonction lire_file ne marche pas c bizar :s
case 1:
puts("Entrez une phrase a crypter");
recup_texte(choix);
break;
case 2:
puts("Entrez le chemin du fichier");
char gc = getchar();
if((gc != '\n')||(gc != '\0')){
fgets(name, 20,stdin);
printf("%s",name);
lire_file(name);
}
break;
default:
puts("Erreur !");
break;
}
return 0;
}
Le fgets est bien pris en compte, deplus il affiche bien le nom du fichier que je rentre mais la fonction lire_file ne marche pas c bizar :s
lirycs78
Messages postés
103
Date d'inscription
vendredi 7 juillet 2006
Statut
Membre
Dernière intervention
7 janvier 2010
1
21 mars 2009 à 19:09
21 mars 2009 à 19:09
Méme en corrigant comme tu me la dit, sa ne marche pas.
Le probléme se situe manifestement au nivo du fopen.
===> le code http://rafb.net/p/n9oUDn74.html
Ps : quand j'ai paste le code je n'avais pas changer ce que tu ma dit, mais méme en le changeant sa ne marche pas :s
Le probléme se situe manifestement au nivo du fopen.
===> le code http://rafb.net/p/n9oUDn74.html
Ps : quand j'ai paste le code je n'avais pas changer ce que tu ma dit, mais méme en le changeant sa ne marche pas :s
Char Snipeur
Messages postés
9813
Date d'inscription
vendredi 23 avril 2004
Statut
Contributeur
Dernière intervention
3 octobre 2023
1 298
21 mars 2009 à 19:39
21 mars 2009 à 19:39
Je ne comprends pas ce passage :
J'aurais tout simplement fait :
Comprends tu exactement ce que tu fais instruction par instruction ?
char gc = getchar(); if((gc != '\n')||(gc != '\0')){ fgets(name, 20,stdin); lire_file(name); }à quoi sert le getchar ?
J'aurais tout simplement fait :
{ fgets(name, 20,stdin); lire_file(name); }
Comprends tu exactement ce que tu fais instruction par instruction ?
lirycs78
Messages postés
103
Date d'inscription
vendredi 7 juillet 2006
Statut
Membre
Dernière intervention
7 janvier 2010
1
21 mars 2009 à 19:45
21 mars 2009 à 19:45
oui, comme il y a un printf ou un puts au dessus du fgets le caracthére \n est ajouter, donc si je ne met pas
if((gc != '\n')||(gc != '\0')){
, Le fgets va prendre automatiquement le '\n' ce qui va donc nous poser un pti soucis :/
if((gc != '\n')||(gc != '\0')){
, Le fgets va prendre automatiquement le '\n' ce qui va donc nous poser un pti soucis :/
lirycs78
Messages postés
103
Date d'inscription
vendredi 7 juillet 2006
Statut
Membre
Dernière intervention
7 janvier 2010
1
22 mars 2009 à 21:38
22 mars 2009 à 21:38
char c;
while((c = getchar()) !='\n' && (c !=EOF));
fgets(name, 20,stdin);
retour_ligne = strchr(name, '\n');
if( retour_ligne == NULL ){
lire_file(name);
}
break;
Jai fai sa il faut juste que je supprime le \n maintenant mai jme rapel plus comment faire :s
while((c = getchar()) !='\n' && (c !=EOF));
fgets(name, 20,stdin);
retour_ligne = strchr(name, '\n');
if( retour_ligne == NULL ){
lire_file(name);
}
break;
Jai fai sa il faut juste que je supprime le \n maintenant mai jme rapel plus comment faire :s
lirycs78
Messages postés
103
Date d'inscription
vendredi 7 juillet 2006
Statut
Membre
Dernière intervention
7 janvier 2010
1
22 mars 2009 à 22:10
22 mars 2009 à 22:10
A non c'est bon sa marche, une ptite derniére question fiddy stp, while((c = getchar()) !='\n' && (c != EOF));
Pk avoir mis " different de EOF " ???
Pk avoir mis " different de EOF " ???
fiddy
Messages postés
11069
Date d'inscription
samedi 5 mai 2007
Statut
Contributeur
Dernière intervention
23 avril 2022
1 844
22 mars 2009 à 22:16
22 mars 2009 à 22:16
Le 'EOF' n'est pas obligatoire. Mais ceci permet de se prémunir du cas où l'utilisateur génère EOF. Disons que c'est la version officielle de vider le buffer clavier en C.