J'ai écrit une fonction qui remplace la virgule par un point dans une chaine de caractères pour ensuite permettre de faire un
calcul.
Pour copier une chaine dans une autre chaine, j'utilise memcpy.
Est ce nécessaire d'écrire
memcpy(ch, c, sizeof(char) * taille);
ou bien je peux me contenter d'écrire
memcpy(ch, c, taille);
Toutes critiques sur la fonction seront les bienvenues.
Merci.
Alain CROS
#include <string.h>
int convstrnum(int taille, char *c)
/* Remplace, dans une chaine de caractères représentant un chiffre,
la virgule par un point.
Retourne :
la position de ce point dans la chaine (base 1).
0 si pas de point.
-1 si pas un chiffre (la chaine reste inchangée).
*/
{
int i = 1, j = 0, k = 0;
char ch[taille + 1];
char *pch = ch;
memcpy(ch, c, sizeof(char) * taille);
ch[taille] = '\0';
while(*pch != '\0')
{
if((*pch < '0') || (*pch > '9'))
switch(*pch)
{
case '.':
j = (++k == 2) ? -1 : j;
break;
case ',':
*pch-- = '.';
j = i--;
break;
default:
j = -1;
}
if(j == -1)
break;
*pch++;
i++;
}
if(j != -1)
memcpy(c, ch, sizeof(char) * taille);
return(j);
}
Non, la suite qui en découle qui est hyper tordue...
Il n'y a pas besoin de créer une chaine locale pour remplacer un caractère dans une chaine modifiable.
-- Emmanuel The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html The C-library: http://www.dinkumware.com/refxc.html
"There are 10 types of people in the world today; those that understand binary, and those that dont."
Harpo
Alain CROS wrote:
int convstrnum(int taille, char *c) /* Remplace, dans une chaine de caractères représentant un chiffre, la virgule par un point. Retourne : la position de ce point dans la chaine (base 1). 0 si pas de point. -1 si pas un chiffre (la chaine reste inchangée). */
Il y a plusieurs choses de bonnes dans cette fonction : 1 ) Tu as explicité son interface en commentaire. 2 ) Cet interface tient la route, retourner le numéro d'un caractère plutôt que son déplacement est un peu choquant (base 1 vs 0), mais c'est compréhensible dans ce contexte. 3 ) Contrairement à ce que certains semblent vouloir dire, c'est très bien d'avoir passé l'adresse de la chaîne et sa longueur, pour cette petite fonction cela n'a pas une grande importance mais c'est généralement très bien de passer la longueur quand on la connaît.
Reste le code, ce n'est pas secondaire. Il est un peu maladroit. 1 ) tes i, j et k. bon, si tu emploies i j et k dans 3 boucles imbriquées, tout le monde comprend mais si tu les additionne et les triture, ça devient difficile à suivre. A chaque variable est attachée une certaine sémantique, il est bon que son nom la reflète, il est également bon que les opérations qui les impliquent la fassent apparaître et que tout le monde (y compris toi dans un an) comprenne ce que ces opérations veulent dire. 2 ) Tu fais 2 memcpy inutiles (1 seul en cas d'erreur), ça coûte très très cher ! 1 axe d'approche est la manière de faire au mieux les choses en étant le plus économe possible. C'est en s'attachant à cela que l'on peut faire de bon programmes. Le plus important est le bon algorithme, mais dans ce cas il se réduit à une boucle de toute manière, donc ce sont les ressources que l'on emploie et non l'algorithme choisi qui est important. Amha, quand tu aura optimisé cette petite fonction, t'aura compris pas mal de choses au C.
Alain CROS wrote:
int convstrnum(int taille, char *c)
/* Remplace, dans une chaine de caractères représentant un chiffre,
la virgule par un point.
Retourne :
la position de ce point dans la chaine (base 1).
0 si pas de point.
-1 si pas un chiffre (la chaine reste inchangée).
*/
Il y a plusieurs choses de bonnes dans cette fonction :
1 ) Tu as explicité son interface en commentaire.
2 ) Cet interface tient la route, retourner le numéro d'un caractère
plutôt que son déplacement est un peu choquant (base 1 vs 0), mais
c'est compréhensible dans ce contexte.
3 ) Contrairement à ce que certains semblent vouloir dire, c'est très
bien d'avoir passé l'adresse de la chaîne et sa longueur, pour cette
petite fonction cela n'a pas une grande importance mais c'est
généralement très bien de passer la longueur quand on la connaît.
Reste le code, ce n'est pas secondaire.
Il est un peu maladroit.
1 ) tes i, j et k. bon, si tu emploies i j et k dans 3 boucles
imbriquées, tout le monde comprend mais si tu les additionne et les
triture, ça devient difficile à suivre.
A chaque variable est attachée une certaine sémantique, il est bon que
son nom la reflète, il est également bon que les opérations qui les
impliquent la fassent apparaître et que tout le monde (y compris toi
dans un an) comprenne ce que ces opérations veulent dire.
2 ) Tu fais 2 memcpy inutiles (1 seul en cas d'erreur), ça coûte très
très cher ! 1 axe d'approche est la manière de faire au mieux les
choses en étant le plus économe possible. C'est en s'attachant à cela
que l'on peut faire de bon programmes.
Le plus important est le bon algorithme, mais dans ce cas il se réduit à
une boucle de toute manière, donc ce sont les ressources que l'on
emploie et non l'algorithme choisi qui est important.
Amha, quand tu aura optimisé cette petite fonction, t'aura compris pas
mal de choses au C.
int convstrnum(int taille, char *c) /* Remplace, dans une chaine de caractères représentant un chiffre, la virgule par un point. Retourne : la position de ce point dans la chaine (base 1). 0 si pas de point. -1 si pas un chiffre (la chaine reste inchangée). */
Il y a plusieurs choses de bonnes dans cette fonction : 1 ) Tu as explicité son interface en commentaire. 2 ) Cet interface tient la route, retourner le numéro d'un caractère plutôt que son déplacement est un peu choquant (base 1 vs 0), mais c'est compréhensible dans ce contexte. 3 ) Contrairement à ce que certains semblent vouloir dire, c'est très bien d'avoir passé l'adresse de la chaîne et sa longueur, pour cette petite fonction cela n'a pas une grande importance mais c'est généralement très bien de passer la longueur quand on la connaît.
Reste le code, ce n'est pas secondaire. Il est un peu maladroit. 1 ) tes i, j et k. bon, si tu emploies i j et k dans 3 boucles imbriquées, tout le monde comprend mais si tu les additionne et les triture, ça devient difficile à suivre. A chaque variable est attachée une certaine sémantique, il est bon que son nom la reflète, il est également bon que les opérations qui les impliquent la fassent apparaître et que tout le monde (y compris toi dans un an) comprenne ce que ces opérations veulent dire. 2 ) Tu fais 2 memcpy inutiles (1 seul en cas d'erreur), ça coûte très très cher ! 1 axe d'approche est la manière de faire au mieux les choses en étant le plus économe possible. C'est en s'attachant à cela que l'on peut faire de bon programmes. Le plus important est le bon algorithme, mais dans ce cas il se réduit à une boucle de toute manière, donc ce sont les ressources que l'on emploie et non l'algorithme choisi qui est important. Amha, quand tu aura optimisé cette petite fonction, t'aura compris pas mal de choses au C.
Alain CROS
Bonjour.
"Harpo" a écrit dans le message de news: 4335b780$0$5400$
| Il y a plusieurs choses de bonnes dans cette fonction :
Merci pour ces encouragements.
| Reste le code, ce n'est pas secondaire. | Il est un peu maladroit. | | Amha, quand tu aura optimisé cette petite fonction, t'aura compris pas | mal de choses au C.
Hum! je crois qu'il y a encore du boulot, mais je ne manquerais pas de revenir ici.
J'ai un peu exploité toutes les réponses, dont certaines comportaient des termes que je n'ai pas compris( VLA , OP). Je pense que ma fonction tiens maintenant un peu mieux la route.
En tous cas, merci à tous.
Alain CROS
#include <ctype.h>
int convstrnum(int taille, char *c) /* Remplace, dans une chaine de caractères représentant un chiffre, la virgule par un point. Retourne : la position de ce point dans la chaine (base 1). 0 si pas de point. -1 si pas un chiffre (la chaine reste inchangée). */ { int i; /*position du char*/ int j = 0; /*position du séparateur décimal*/ char *pch = c; for(i = 1; i <= taille; i++) { if(!isdigit(*pch)) switch(*pch) { case ',': case '.': /*si plusieurs virgules et ou points --> erreur*/ j = (j == 0) ? i : -1; break; case '-': /*nombre négatif*/ if(i == 1) break; default: /*mauvais caractère --> erreur*/ j = -1; } if(j == -1) break; *pch++; } if(j > 0) *( c + j - 1 ) = '.' ; /*le séparateur décimal devient un point*/ return j; }
Bonjour.
"Harpo" <trashcan@hotmail.com> a écrit dans le message de news: 4335b780$0$5400$8fcfb975@news.wanadoo.fr...
| Il y a plusieurs choses de bonnes dans cette fonction :
Merci pour ces encouragements.
| Reste le code, ce n'est pas secondaire.
| Il est un peu maladroit.
|
| Amha, quand tu aura optimisé cette petite fonction, t'aura compris pas
| mal de choses au C.
Hum! je crois qu'il y a encore du boulot, mais je ne manquerais pas de revenir ici.
J'ai un peu exploité toutes les réponses, dont certaines comportaient des termes que je n'ai pas compris( VLA , OP).
Je pense que ma fonction tiens maintenant un peu mieux la route.
En tous cas, merci à tous.
Alain CROS
#include <ctype.h>
int convstrnum(int taille, char *c)
/* Remplace, dans une chaine de caractères représentant un chiffre,
la virgule par un point.
Retourne :
la position de ce point dans la chaine (base 1).
0 si pas de point.
-1 si pas un chiffre (la chaine reste inchangée).
*/
{
int i; /*position du char*/
int j = 0; /*position du séparateur décimal*/
char *pch = c;
for(i = 1; i <= taille; i++)
{
if(!isdigit(*pch))
switch(*pch)
{
case ',':
case '.': /*si plusieurs virgules et ou points --> erreur*/
j = (j == 0) ? i : -1;
break;
case '-': /*nombre négatif*/
if(i == 1)
break;
default: /*mauvais caractère --> erreur*/
j = -1;
}
if(j == -1)
break;
*pch++;
}
if(j > 0)
*( c + j - 1 ) = '.' ; /*le séparateur décimal devient un point*/
return j;
}
"Harpo" a écrit dans le message de news: 4335b780$0$5400$
| Il y a plusieurs choses de bonnes dans cette fonction :
Merci pour ces encouragements.
| Reste le code, ce n'est pas secondaire. | Il est un peu maladroit. | | Amha, quand tu aura optimisé cette petite fonction, t'aura compris pas | mal de choses au C.
Hum! je crois qu'il y a encore du boulot, mais je ne manquerais pas de revenir ici.
J'ai un peu exploité toutes les réponses, dont certaines comportaient des termes que je n'ai pas compris( VLA , OP). Je pense que ma fonction tiens maintenant un peu mieux la route.
En tous cas, merci à tous.
Alain CROS
#include <ctype.h>
int convstrnum(int taille, char *c) /* Remplace, dans une chaine de caractères représentant un chiffre, la virgule par un point. Retourne : la position de ce point dans la chaine (base 1). 0 si pas de point. -1 si pas un chiffre (la chaine reste inchangée). */ { int i; /*position du char*/ int j = 0; /*position du séparateur décimal*/ char *pch = c; for(i = 1; i <= taille; i++) { if(!isdigit(*pch)) switch(*pch) { case ',': case '.': /*si plusieurs virgules et ou points --> erreur*/ j = (j == 0) ? i : -1; break; case '-': /*nombre négatif*/ if(i == 1) break; default: /*mauvais caractère --> erreur*/ j = -1; } if(j == -1) break; *pch++; } if(j > 0) *( c + j - 1 ) = '.' ; /*le séparateur décimal devient un point*/ return j; }
Emmanuel Delahaye
Alain CROS wrote on 25/09/05 :
J'ai un peu exploité toutes les réponses, dont certaines comportaient des termes que je n'ai pas compris( VLA , OP).
VLA : Variable Length Array. Une nouvelle possibilité de C99. A ma connaissance, il n'y a pas d'implémentation conforme des VLA.
OP : Original Poster (Questionneur initial)
Je pense que ma fonction tiens maintenant un peu mieux la route.
#include <ctype.h>
<...>
Pourquoi est-ce aussi compliqué avec des +1 et des -1 dans tous les sens. Il suffit d'utiliser un index basé en 0 comme il se doit et c'est tout.
-- Emmanuel The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html The C-library: http://www.dinkumware.com/refxc.html
"It's specified. But anyone who writes code like that should be transmogrified into earthworms and fed to ducks." -- Chris Dollin CLC
Alain CROS wrote on 25/09/05 :
J'ai un peu exploité toutes les réponses, dont certaines comportaient des
termes que je n'ai pas compris( VLA , OP).
VLA : Variable Length Array. Une nouvelle possibilité de C99. A ma
connaissance, il n'y a pas d'implémentation conforme des VLA.
OP : Original Poster (Questionneur initial)
Je pense que ma fonction tiens
maintenant un peu mieux la route.
#include <ctype.h>
<...>
Pourquoi est-ce aussi compliqué avec des +1 et des -1 dans tous les
sens. Il suffit d'utiliser un index basé en 0 comme il se doit et c'est
tout.
--
Emmanuel
The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html
The C-library: http://www.dinkumware.com/refxc.html
"It's specified. But anyone who writes code like that should be
transmogrified into earthworms and fed to ducks." -- Chris Dollin CLC
A 399$ la version Linux, on pouvait effectivement s'attendre à ce que ça fonctionne...
Il reste gratuit pour une utilisation non commerciale.
-- Richard
Charlie Gordon
"Emmanuel Delahaye" wrote in message news:
(supersedes )
Richard Delorme wrote on 25/09/05 :
VLA : Variable Length Array. Une nouvelle possibilité de C99. A ma connaissance, il n'y a pas d'implémentation conforme des VLA.
Donc soit Intel ment, soit Emmanuel Delahaye a des connaissances limitées : http://minilien.com/?PJReJeywMm
Je connais la réponse !
A 399$ la version Linux, on pouvait effectivement s'attendre à ce que ça fonctionne...
Le fait qu'ils les supporte n'implique pas necessairement que l'implementation soit conforme, d'autant qu'il mettent en avant la compatibilité source avec gcc dont justement le support des VLA pose probleme... mais admettons. Deja 399 USD, c'est pas cher pour un outil pro, mais intel permet de telecharger gratuitement ce compilo pour un usage personnel (si vous developpez des trucs sur votre temps non remunéré ;-): http://www.intel.com/cd/software/products/asmo-na/eng/compilers/clin/219771.htm Faudra regarder si leurs pretentions en termes d'optimisations sont justifiees. Ils citent essentiellement gcc comme reference, qui n'est pas super fort en optimisations, surtout au niveau du code generé pour des architectures particulieres, alors qu'on peut imaginer qu'un compilateur produit par intel n'a que cela comme objectif.
-- Chqrlie.
"Emmanuel Delahaye" <emdel@YOURBRAnoos.fr> wrote in message
news:mn.cd767d591baf1ae0.15512@YOURBRAnoos.fr...
VLA : Variable Length Array. Une nouvelle possibilité de C99. A ma
connaissance, il n'y a pas d'implémentation conforme des VLA.
Donc soit Intel ment, soit Emmanuel Delahaye a des connaissances limitées :
http://minilien.com/?PJReJeywMm
Je connais la réponse !
A 399$ la version Linux, on pouvait effectivement s'attendre à ce que
ça fonctionne...
Le fait qu'ils les supporte n'implique pas necessairement que l'implementation
soit conforme, d'autant qu'il mettent en avant la compatibilité source avec gcc
dont justement le support des VLA pose probleme... mais admettons.
Deja 399 USD, c'est pas cher pour un outil pro, mais intel permet de telecharger
gratuitement ce compilo pour un usage personnel (si vous developpez des trucs
sur votre temps non remunéré ;-):
http://www.intel.com/cd/software/products/asmo-na/eng/compilers/clin/219771.htm
Faudra regarder si leurs pretentions en termes d'optimisations sont justifiees.
Ils citent essentiellement gcc comme reference, qui n'est pas super fort en
optimisations, surtout au niveau du code generé pour des architectures
particulieres, alors qu'on peut imaginer qu'un compilateur produit par intel n'a
que cela comme objectif.
VLA : Variable Length Array. Une nouvelle possibilité de C99. A ma connaissance, il n'y a pas d'implémentation conforme des VLA.
Donc soit Intel ment, soit Emmanuel Delahaye a des connaissances limitées : http://minilien.com/?PJReJeywMm
Je connais la réponse !
A 399$ la version Linux, on pouvait effectivement s'attendre à ce que ça fonctionne...
Le fait qu'ils les supporte n'implique pas necessairement que l'implementation soit conforme, d'autant qu'il mettent en avant la compatibilité source avec gcc dont justement le support des VLA pose probleme... mais admettons. Deja 399 USD, c'est pas cher pour un outil pro, mais intel permet de telecharger gratuitement ce compilo pour un usage personnel (si vous developpez des trucs sur votre temps non remunéré ;-): http://www.intel.com/cd/software/products/asmo-na/eng/compilers/clin/219771.htm Faudra regarder si leurs pretentions en termes d'optimisations sont justifiees. Ils citent essentiellement gcc comme reference, qui n'est pas super fort en optimisations, surtout au niveau du code generé pour des architectures particulieres, alors qu'on peut imaginer qu'un compilateur produit par intel n'a que cela comme objectif.
-- Chqrlie.
Charlie Gordon
"Alain CROS" wrote in message news:4336f07f$0$18817$
En tous cas, merci à tous.
Voici mon analyse de la derniere version de ton code :
#include <ctype.h>
int convstrnum(int taille, char *c) /* Remplace, dans une chaine de caractères représentant un chiffre, la virgule par un point. Retourne : la position de ce point dans la chaine (base 1). 0 si pas de point. -1 si pas un chiffre (la chaine reste inchangée). */
- Je mettrais le commentaire au dessus de la definition de la fonction, pas au milieu. - La chaine ne represente pas un chiffre, mais un nombre, essaie d'être précis dans les explications. - c n'est pas un nom loisible pour un char *, utilise p, s, ou str. - taille est un mot francais : essaie de ne pas melanger francais et anglais dans le code, comme les mots-clés et les noms de fonction sont deja en anglais (y compris convstrnum) tu devrais utiliser len pour ce parametre. - convstrnum n'est pas non plus très cohérent comme nom : que penses-tu de normalize_number_string ?
{ int i; /*position du char*/ int j = 0; /*position du séparateur décimal*/ char *pch = c; for(i = 1; i <= taille; i++)
Cette boucle est à bannir dans un programme C. La facon standard de faire une telle boucle est
for(i = 0; i < len; i++)
dès que tu t'écartes des constructions habituelles, le code devient difficile à suivre, et les erreurs aux bornes rappliquent rapidement. En particulier dans cette boucle, i n'est *pas* la position du caractère pointé par pch, donc le commentaire est faux, ce qui indique souvent que le code l'est aussi.
{ if(!isdigit(*pch)) switch(*pch) { case ',': case '.': /*si plusieurs virgules et ou points --> erreur*/ j = (j == 0) ? i : -1;
l'utilisation de l'opérateur ternaire rend ici le code obscur. il serait plus facile a comprendre avec un if qui teste si on a deja vu un séparateur décimal et revoie le code d'erreur dans ce cas plutot que de stocker -1 dans j
break; case '-': /*nombre négatif*/ if(i == 1) break;
Il manque un commentaire /* FALL THRU */ ici et une explication pour le refus des nombres comprenant un signe - ailleurs qu'au début. tu peux traiter ce cas plus efficacement et plus lisiblement avant la boucle.
default: /*mauvais caractère --> erreur*/ j = -1;
manque encore un break. celui-ci n'est pas strictement indispensable, mais il est fortement recommandé de toujours terminer une clause de switch par un break pour améliorer la lisibilité et éviter les erreurs lors de rajout de clauses supplémentaires.
} if(j == -1) break; *pch++;
pourquoi déréférencer pch ici ?
} if(j > 0) *( c + j - 1 ) = '.' ; /*le séparateur décimal devient un point*/
c[j - 1] = '.'; est plus lisible à mon sens.
return j; }
Remarques générales :
Si cette fonction n'est pas appelée pour des sous-parties de chaine, il serait plus simple de lui passer seulement le pointeur et de boucler jusqu'au ' ' de fin de chaine. Retourner un nombre prétendument 'position de la virgule' basé en 1 est une hérésie à éviter *absolument* en C. Tot ou tard on fera la supposition que str[pos] == '.' ou le code sera anormalement alourdi par des -1 partout. Renvoie plutot le nombre de caractères devant le point et -1 ou len si le nombre n'a pas de point, et -2 en cas de caractères interdits. Tu pourrais aussi accepter des espaces optionnelles au début de la chaine, comme le font les routines standard de conversion de nombres (strtol, atoi, strtod) Tu devrais aussi accepter le signe +
Voici une proposition:
#include <ctype.h>
int normalize_number_string(char *s) { char *p; char *pdec;
/* on accepte les pointeurs NULL, mais c'est un cas d'erreur */ if (s == NULL) return -2;
p = s; /* on ignore les blancs en début de chaine */ while (isspace(*p)) p++; /* le nombre peut commencer par un signe */ if (*p == '+' || *p == '-') p++; /* on reconnait la partie entiere */ while (isdigit(*p)) p++; if (*p == ' ') { /* a chaine est correcte, mais n'a pas de point decimal */ return -1; } if (*p != ',' && *p != '.') { /* la chaine n'est pas un nombre */ return -2; } /* on garde la position du point decimal pour ne modifier la chaine que si celle-ci est correcte */ pdec = p++; /* on reconnait la partie décimale */ while (isdigit(*p)) p++; if (*p == ' ') { /* la chaine est correcte, on force le point decimal et retourne sa position */ *pdec = '.'; return (pdec - s); } else { /* la chaine est incorrecte, on la laisse inchangée */ return -2; } }
Si tu tiens absolument à passer len en parametre, le code sera nettement plus lourd, et on sera tenté de ne faire qu'une seule boucle, ce qui le rendra moins lisible. Si tu veux conserver la sémantique initiale, il suffit de modifier les return.
Tu remarqueras que cette implementation te permet d'affiner facilement le comportement pour renvoyer des codes d'erreur plus explicites, detecter des nombres potentiellement incorrects : une partie entiere ou une partie decimale vide sont sans doute corrects, mais que dire de la chaine "." ? Tu peux aussi facilement changer la sémantique de cette fonction pour qu'elle serve a analyser le début d'une chaine et positionne un pointeur sur le caractère qui suit le nombre reconnu, comme le fait strtol(). Ce comportement-là est beaucoup plus utile en pratique que le code d'erreur retourné dans l'API actuelle. On aurait alors une fonction :
int parse_number_string(char *s, char **tailp);
qui positionne tailp sur la suite de la chaine et renvoie un code indiquant le type de nombre reconnu : pas un nombre, nombre entier, nombre avec un point décimal...
-- Chqrlie.
"Alain CROS" <Personne@ICI> wrote in message
news:4336f07f$0$18817$626a14ce@news.free.fr...
En tous cas, merci à tous.
Voici mon analyse de la derniere version de ton code :
#include <ctype.h>
int convstrnum(int taille, char *c)
/* Remplace, dans une chaine de caractères représentant un chiffre,
la virgule par un point.
Retourne :
la position de ce point dans la chaine (base 1).
0 si pas de point.
-1 si pas un chiffre (la chaine reste inchangée).
*/
- Je mettrais le commentaire au dessus de la definition de la fonction, pas au
milieu.
- La chaine ne represente pas un chiffre, mais un nombre, essaie d'être précis
dans les explications.
- c n'est pas un nom loisible pour un char *, utilise p, s, ou str.
- taille est un mot francais : essaie de ne pas melanger francais et anglais
dans le code, comme les mots-clés et les noms de fonction sont deja en anglais
(y compris convstrnum) tu devrais utiliser len pour ce parametre.
- convstrnum n'est pas non plus très cohérent comme nom : que penses-tu de
normalize_number_string ?
{
int i; /*position du char*/
int j = 0; /*position du séparateur décimal*/
char *pch = c;
for(i = 1; i <= taille; i++)
Cette boucle est à bannir dans un programme C. La facon standard de faire une
telle boucle est
for(i = 0; i < len; i++)
dès que tu t'écartes des constructions habituelles, le code devient difficile à
suivre, et les erreurs aux bornes rappliquent rapidement. En particulier dans
cette boucle, i n'est *pas* la position du caractère pointé par pch, donc le
commentaire est faux, ce qui indique souvent que le code l'est aussi.
{
if(!isdigit(*pch))
switch(*pch)
{
case ',':
case '.': /*si plusieurs virgules et ou points --> erreur*/
j = (j == 0) ? i : -1;
l'utilisation de l'opérateur ternaire rend ici le code obscur. il serait plus
facile a comprendre avec un if qui teste si on a deja vu un séparateur décimal
et revoie le code d'erreur dans ce cas plutot que de stocker -1 dans j
break;
case '-': /*nombre négatif*/
if(i == 1)
break;
Il manque un commentaire /* FALL THRU */ ici et une explication pour le refus
des nombres comprenant un signe - ailleurs qu'au début.
tu peux traiter ce cas plus efficacement et plus lisiblement avant la boucle.
default: /*mauvais caractère --> erreur*/
j = -1;
manque encore un break. celui-ci n'est pas strictement indispensable, mais il
est fortement recommandé de toujours terminer une clause de switch par un break
pour améliorer la lisibilité et éviter les erreurs lors de rajout de clauses
supplémentaires.
}
if(j == -1)
break;
*pch++;
pourquoi déréférencer pch ici ?
}
if(j > 0)
*( c + j - 1 ) = '.' ; /*le séparateur décimal devient un point*/
c[j - 1] = '.'; est plus lisible à mon sens.
return j;
}
Remarques générales :
Si cette fonction n'est pas appelée pour des sous-parties de chaine, il serait
plus simple de lui passer seulement le pointeur et de boucler jusqu'au ' ' de
fin de chaine.
Retourner un nombre prétendument 'position de la virgule' basé en 1 est une
hérésie à éviter *absolument* en C. Tot ou tard on fera la supposition que
str[pos] == '.' ou le code sera anormalement alourdi par des -1 partout.
Renvoie plutot le nombre de caractères devant le point et -1 ou len si le nombre
n'a pas de point, et -2 en cas de caractères interdits.
Tu pourrais aussi accepter des espaces optionnelles au début de la chaine, comme
le font les routines standard de conversion de nombres (strtol, atoi, strtod)
Tu devrais aussi accepter le signe +
Voici une proposition:
#include <ctype.h>
int normalize_number_string(char *s) {
char *p;
char *pdec;
/* on accepte les pointeurs NULL, mais c'est un cas d'erreur */
if (s == NULL)
return -2;
p = s;
/* on ignore les blancs en début de chaine */
while (isspace(*p))
p++;
/* le nombre peut commencer par un signe */
if (*p == '+' || *p == '-')
p++;
/* on reconnait la partie entiere */
while (isdigit(*p))
p++;
if (*p == ' ') {
/* a chaine est correcte, mais n'a pas de point decimal */
return -1;
}
if (*p != ',' && *p != '.') {
/* la chaine n'est pas un nombre */
return -2;
}
/* on garde la position du point decimal pour ne modifier la chaine que si
celle-ci est correcte */
pdec = p++;
/* on reconnait la partie décimale */
while (isdigit(*p))
p++;
if (*p == ' ') {
/* la chaine est correcte, on force le point decimal et retourne sa
position */
*pdec = '.';
return (pdec - s);
} else {
/* la chaine est incorrecte, on la laisse inchangée */
return -2;
}
}
Si tu tiens absolument à passer len en parametre, le code sera nettement plus
lourd, et on sera tenté de ne faire qu'une seule boucle, ce qui le rendra moins
lisible.
Si tu veux conserver la sémantique initiale, il suffit de modifier les return.
Tu remarqueras que cette implementation te permet d'affiner facilement le
comportement pour renvoyer des codes d'erreur plus explicites, detecter des
nombres potentiellement incorrects : une partie entiere ou une partie decimale
vide sont sans doute corrects, mais que dire de la chaine "." ?
Tu peux aussi facilement changer la sémantique de cette fonction pour qu'elle
serve a analyser le début d'une chaine et positionne un pointeur sur le
caractère qui suit le nombre reconnu, comme le fait strtol(). Ce
comportement-là est beaucoup plus utile en pratique que le code d'erreur
retourné dans l'API actuelle. On aurait alors une fonction :
int parse_number_string(char *s, char **tailp);
qui positionne tailp sur la suite de la chaine et renvoie un code indiquant le
type de nombre reconnu : pas un nombre, nombre entier, nombre avec un point
décimal...
"Alain CROS" wrote in message news:4336f07f$0$18817$
En tous cas, merci à tous.
Voici mon analyse de la derniere version de ton code :
#include <ctype.h>
int convstrnum(int taille, char *c) /* Remplace, dans une chaine de caractères représentant un chiffre, la virgule par un point. Retourne : la position de ce point dans la chaine (base 1). 0 si pas de point. -1 si pas un chiffre (la chaine reste inchangée). */
- Je mettrais le commentaire au dessus de la definition de la fonction, pas au milieu. - La chaine ne represente pas un chiffre, mais un nombre, essaie d'être précis dans les explications. - c n'est pas un nom loisible pour un char *, utilise p, s, ou str. - taille est un mot francais : essaie de ne pas melanger francais et anglais dans le code, comme les mots-clés et les noms de fonction sont deja en anglais (y compris convstrnum) tu devrais utiliser len pour ce parametre. - convstrnum n'est pas non plus très cohérent comme nom : que penses-tu de normalize_number_string ?
{ int i; /*position du char*/ int j = 0; /*position du séparateur décimal*/ char *pch = c; for(i = 1; i <= taille; i++)
Cette boucle est à bannir dans un programme C. La facon standard de faire une telle boucle est
for(i = 0; i < len; i++)
dès que tu t'écartes des constructions habituelles, le code devient difficile à suivre, et les erreurs aux bornes rappliquent rapidement. En particulier dans cette boucle, i n'est *pas* la position du caractère pointé par pch, donc le commentaire est faux, ce qui indique souvent que le code l'est aussi.
{ if(!isdigit(*pch)) switch(*pch) { case ',': case '.': /*si plusieurs virgules et ou points --> erreur*/ j = (j == 0) ? i : -1;
l'utilisation de l'opérateur ternaire rend ici le code obscur. il serait plus facile a comprendre avec un if qui teste si on a deja vu un séparateur décimal et revoie le code d'erreur dans ce cas plutot que de stocker -1 dans j
break; case '-': /*nombre négatif*/ if(i == 1) break;
Il manque un commentaire /* FALL THRU */ ici et une explication pour le refus des nombres comprenant un signe - ailleurs qu'au début. tu peux traiter ce cas plus efficacement et plus lisiblement avant la boucle.
default: /*mauvais caractère --> erreur*/ j = -1;
manque encore un break. celui-ci n'est pas strictement indispensable, mais il est fortement recommandé de toujours terminer une clause de switch par un break pour améliorer la lisibilité et éviter les erreurs lors de rajout de clauses supplémentaires.
} if(j == -1) break; *pch++;
pourquoi déréférencer pch ici ?
} if(j > 0) *( c + j - 1 ) = '.' ; /*le séparateur décimal devient un point*/
c[j - 1] = '.'; est plus lisible à mon sens.
return j; }
Remarques générales :
Si cette fonction n'est pas appelée pour des sous-parties de chaine, il serait plus simple de lui passer seulement le pointeur et de boucler jusqu'au ' ' de fin de chaine. Retourner un nombre prétendument 'position de la virgule' basé en 1 est une hérésie à éviter *absolument* en C. Tot ou tard on fera la supposition que str[pos] == '.' ou le code sera anormalement alourdi par des -1 partout. Renvoie plutot le nombre de caractères devant le point et -1 ou len si le nombre n'a pas de point, et -2 en cas de caractères interdits. Tu pourrais aussi accepter des espaces optionnelles au début de la chaine, comme le font les routines standard de conversion de nombres (strtol, atoi, strtod) Tu devrais aussi accepter le signe +
Voici une proposition:
#include <ctype.h>
int normalize_number_string(char *s) { char *p; char *pdec;
/* on accepte les pointeurs NULL, mais c'est un cas d'erreur */ if (s == NULL) return -2;
p = s; /* on ignore les blancs en début de chaine */ while (isspace(*p)) p++; /* le nombre peut commencer par un signe */ if (*p == '+' || *p == '-') p++; /* on reconnait la partie entiere */ while (isdigit(*p)) p++; if (*p == ' ') { /* a chaine est correcte, mais n'a pas de point decimal */ return -1; } if (*p != ',' && *p != '.') { /* la chaine n'est pas un nombre */ return -2; } /* on garde la position du point decimal pour ne modifier la chaine que si celle-ci est correcte */ pdec = p++; /* on reconnait la partie décimale */ while (isdigit(*p)) p++; if (*p == ' ') { /* la chaine est correcte, on force le point decimal et retourne sa position */ *pdec = '.'; return (pdec - s); } else { /* la chaine est incorrecte, on la laisse inchangée */ return -2; } }
Si tu tiens absolument à passer len en parametre, le code sera nettement plus lourd, et on sera tenté de ne faire qu'une seule boucle, ce qui le rendra moins lisible. Si tu veux conserver la sémantique initiale, il suffit de modifier les return.
Tu remarqueras que cette implementation te permet d'affiner facilement le comportement pour renvoyer des codes d'erreur plus explicites, detecter des nombres potentiellement incorrects : une partie entiere ou une partie decimale vide sont sans doute corrects, mais que dire de la chaine "." ? Tu peux aussi facilement changer la sémantique de cette fonction pour qu'elle serve a analyser le début d'une chaine et positionne un pointeur sur le caractère qui suit le nombre reconnu, comme le fait strtol(). Ce comportement-là est beaucoup plus utile en pratique que le code d'erreur retourné dans l'API actuelle. On aurait alors une fonction :
int parse_number_string(char *s, char **tailp);
qui positionne tailp sur la suite de la chaine et renvoie un code indiquant le type de nombre reconnu : pas un nombre, nombre entier, nombre avec un point décimal...
