Twitter iPhone pliant OnePlus 11 PS5 Disney+ Orange Livebox Windows 11

Revue de code, si vous avez des idées pour mieux écrire / optimiser le bouzin

7 réponses
Avatar
Targeur fou
Bonjour,

J'ai pondu un petit module de seuillage de points (ex utilisation :
clipping de courbes) qui marche bien. Ceci dit, bien que j'arrive =E0
coder des algos de fa=E7on na=EFve et arriver =E0 les faire tourner
correctement, je suis toujours aussi nul pour les =E9crire de la fa=E7on
la plus efficace qui soit (je parle d'=E9criture optimale en C, pas
d'algo, mais si toutefois vous avez des remarques dessus aussi, je suis
preneur).

Si vous voulez bien jeter un oeil sur le code et proposer des
am=E9liorations :

Fichiers ici (format Unix) :

seuillage.h : http://cjoint.com/?bunNNrwqnG
seuillage.c : http://cjoint.com/?bunOkyyYSs

Merci,

A+
Regis

7 réponses

Avatar
Stephane Legras-Decussy
"Targeur fou" a écrit dans le message de news:

Si vous voulez bien jeter un oeil sur le code et proposer des
améliorations :


alors si c'est pour apprendre à faire des gros projets, ok
mais sinon les fonctionnalités du module me semble derisoire
par rapport à la taille et à la lisibilité de l'ensemble...

en fait on comprend rien quand le code est noyés
dans les commentaires. les commentaires ne doivent
pas paraphraser le code.

les noms de variables sont trop long par rapport à leur portée.

je ferais un petit blabla d'un seul bloc avec un dico des variables au début
et après c'est parti pour coder avec i, j, k en 15 lignes.

et le coup du "moteur" et "init_moteur", faut reserver ça à des vrai trucs
importants, sinon va y avoir 250 moteurs dans le programmes...

Avatar
Targeur fou

Salut,

"Targeur fou" a écrit dans le message de news:

Si vous voulez bien jeter un oeil sur le code et proposer des
améliorations :


alors si c'est pour apprendre à faire des gros projets, ok
mais sinon les fonctionnalités du module me semble derisoire
par rapport à la taille et à la lisibilité de l'ensemble...


Oui c'est de toute façon un gros projet. Ce n'est qu'un extrait de
fichiers plus gros en vrais.

en fait on comprend rien quand le code est noyés
dans les commentaires. les commentaires ne doivent
pas paraphraser le code.


Tu ne connais pas le niveau moyen en C d'un programmeur de SSII ;-) Il
faut parfois du vrai code de neuneu et paraphraser le code avec les
commentaires. Je suis d'accord que j'ai exagéré et que c'est
lourdingue. Je me fais pardonner plus bas avec le minimum C compilable.

les noms de variables sont trop long par rapport à leur portée.


Là je ne te suis pas, les noms des membres de la structure sont
lourdingues et les noms de paramètres aussi, mais pas les variables
locales.

et le coup du "moteur" et "init_moteur", faut reserver ça à des vrai trucs
importants, sinon va y avoir 250 moteurs dans le programmes...


C'est quoi un vrai truc important. Il s'agissait de MoteurSeuillage,
"Moteur" signifiant en fait structure d'état ici, c'est dur de choisir
des noms pour des types et variables. J'ai changé en Etat pour te
faire plaisir ;-)

Voilà le bouzin modifié qui annule le précédent ( de toute façon,
il y avait un gros bug dans seuillage.c dû à un copier-coller (un
cast...) ) :

_____________________________________________

/* en-tête pour strcmp() et strcpy() */
#include <string.h>

/* Type enum pour type de seuillage */
typedef enum
{
HAUT = 0, /* Seuillage au-dessus d'une valeur) */
BAS /* Seuillage en dessous d'une valeur) */
} TypeSeuillage_e;

/* Type struct pour point */
typedef struct
{
double x; /* Abscisse */
double y; /* Ordonnée */
} PointDouble_s;

/* Type struct pour etat de seuillage */
typedef struct
{
TypeSeuillage_e type; /* Type de seuil */
double seuil; /* Valeur du seuil */
int depSeuilPrec; /* Drapeau depassement de seuil du point
precedent */
int depSeuil; /* Drapeau depassement de seuil du point
courant */
PointDouble_s ptPrec; /* Point précédent */
PointDouble_s pt; /* Point courant */
char tabTxtInit[5]; /* Texte drapeau d'initialisation */
} EtatSeuillageDbl_s;

/* Type enum pour erreur de seuillage */
typedef enum
{
ETAT_NON_INIT = -20000, /* Etat non initialise */
CD_DIV_ZERO /* Division par zéro */
} ErrSeuillage_e;

/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/

int initEtatSeuillageDbl(EtatSeuillageDbl_s * pEtat, TypeSeuillage_e
type, double seuil)
{
int rc = 0;
if (pEtat == NULL) { rc = -1; }
if (rc >= 0) {
pEtat->type = type;
pEtat->seuil = seuil;
pEtat->depSeuilPrec = pEtat->depSeuil = 0;
pEtat->ptPrec.x = pEtat->ptPrec.y = 0.0;
pEtat->pt.x = pEtat->pt.y = 0.0;
strcpy(pEtat->tabTxtInit, "init");
}
return rc;
}

/*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/

int seuillerPtsDbl(EtatSeuillageDbl_s * pEtat,
const PointDouble_s * src,
const size_t nbSrc,
PointDouble_s * dst,
size_t * pNbDst)
{
int rc = 0;
if (pEtat == NULL) { rc = -1; }
if (src == NULL) { rc = -2; }
if (nbSrc == 0u) { rc = -3; }
if (dst == 0) { rc = -4; }
if (pNbDst == NULL || *pNbDst < 2*nbSrc ) { rc = -5; }
if (strcmp(pEtat->tabTxtInit, "init") != 0) { rc = ETAT_NON_INIT; }

if (rc >= 0) {
if (nbSrc == 1) {
dst[0].x = src[0].x;
dst[0].y = ( (pEtat->type == HAUT && src[0].y > pEtat->seuil)
||
(pEtat->type == BAS && src[0].y < pEtat->seuil) )
?
pEtat->seuil : dst[0].y;
}
}
else {

size_t i, j;
double cd;

for (i=0u, j=0u; i < nbSrc; ++i, ++j) {
pEtat->pt = dst[j] = src[i];
/* MAJ flag de depassement de seuil du point courant */
pEtat->depSeuil = ( (pEtat->type == HAUT && pEtat->pt.y >
pEtat->seuil) ||
(pEtat->type == BAS && pEtat->pt.y <
pEtat->seuil) ) ?
1 : 0;
if (i == 0u) {
if (pEtat->depSeuil) {
dst[j].y = pEtat->seuil;
}
}
else {

if ((pEtat->pt.x - pEtat->ptPrec.x) == 0.0) {
rc = CD_DIV_ZERO;
/* RAZ structure d'etat sur erreur */
pEtat->depSeuilPrec = pEtat->depSeuil = 0;
return rc;
}

/* Coefficient directeur entre pt courant et pt precedent
*/
cd = (pEtat->pt.y - pEtat->ptPrec.y)/(pEtat->pt.x -
pEtat->ptPrec.x);

/* Cas franchissement de seuil du pt courant */
if ( (pEtat->depSeuil && !pEtat->depSeuilPrec) ||
(!pEtat->depSeuil && pEtat->depSeuilPrec) ) {
dst[j].x = pEtat->ptPrec.x + (pEtat->seuil -
pEtat->ptPrec.y)/cd;
dst[j].y = pEtat->seuil;
++j;
dst[j] = pEtat->pt;
if (pEtat->depSeuil) {
dst[j].y = pEtat->seuil;
}
}
else if (pEtat->depSeuil) {
dst[j].y = pEtat->seuil;
}
}
pEtat->ptPrec = pEtat->pt;
pEtat->depSeuilPrec = pEtat->depSeuil;
}

/* MAJ taille reelle tampon destination */
*pNbDst = j;
/* RAZ structure d'etat pour prochaine utilisation */
pEtat->depSeuilPrec = pEtat->depSeuil = 0;
}

return rc;
}

A+
Regis


Avatar
Harpo
Targeur fou wrote:

Tu ne connais pas le niveau moyen en C d'un programmeur de SSII ;-)


Il m'est arrivé d'en être un.


Voilà le bouzin modifié qui annule le précédent ( de toute façon,
il y avait un gros bug dans seuillage.c dû à un copier-coller (un
cast...) ) :


ça semble correct à première vue.
Juste une remarque :

int seuillerPtsDbl(EtatSeuillageDbl_s * pEtat,
const PointDouble_s * src,
const size_t nbSrc,
PointDouble_s * dst,
size_t * pNbDst)


Je pense que 3 arguments à une fonction est en général un maximum, avec
plus on se demande parfois dans quel ordre les mettre.
Cette règle ne s'applique pas vraiment ici car src et dst sont presque
symétrique, cela correspond à une fonction qui aurait 3,5 arguments, ça
passe.

--
http://harpo.free.fr/

Avatar
Stephane Legras-Decussy
"Targeur fou" a écrit dans le message de news:


C'est quoi un vrai truc important.


je sais pas, un moteur d'affichage 3D par exemple, on voit des trucs comme
ça : init_le_bouzin et ensuite run_le_bouzin

Voilà le bouzin modifié qui annule le précédent ( de toute façon,
il y avait un gros bug dans seuillage.c dû à un copier-coller (un
cast...) ) :


ok. ça correspond beaucoup à ce que j'aurais envie de relire... :-)

Avatar
Targeur fou
Harpo wrote:

Salut,

Targeur fou wrote:

Tu ne connais pas le niveau moyen en C d'un programmeur de SSII ;-)


Il m'est arrivé d'en être un.


A ne pas prendre pour une généralité bien sûr, mais il est vrai que
le fait de travailler en SSII fait que pour beaucoup de programmeurs de
ce milieu, il changent souvent de technos/langages comme de domaines
fonctionnels, et disposent par conséquent d'une bonne connaissance
globale, mais sont parfois très limités dans un domaine qu'ils ont
juste eu le temps d'appréhender. Et encore, il y en a même avec
quelques années de partique qui à force de contraintes coûts/délais
assez fortes n'ont jamais pris ni ne prennent pas le temps de bien
faire les choses.


Voilà le bouzin modifié qui annule le précédent ( de toute fa çon,
il y avait un gros bug dans seuillage.c dû à un copier-coller (un
cast...) ) :


ça semble correct à première vue.
Juste une remarque :

int seuillerPtsDbl(EtatSeuillageDbl_s * pEtat,
const PointDouble_s * src,
const size_t nbSrc,
PointDouble_s * dst,
size_t * pNbDst)


Je pense que 3 arguments à une fonction est en général un maximum, avec
plus on se demande parfois dans quel ordre les mettre.
Cette règle ne s'applique pas vraiment ici car src et dst sont presque
symétrique, cela correspond à une fonction qui aurait 3,5 arguments, ça
passe.



OK, merci.

A+
Regis


Avatar
Targeur fou
Stephane Legras-Decussy wrote:

Salut,

"Targeur fou" a écrit dans le message de news:


C'est quoi un vrai truc important.


je sais pas, un moteur d'affichage 3D par exemple, on voit des trucs comme
ça : init_le_bouzin et ensuite run_le_bouzin


Bon, bon, c'est sûr, la bibliothèque au final ne sera pas non plus de
l'ampleur d'un moteur d'affichage 3D ;-) Mais là j'ai quand même
gardé les fonctions init() et run() car je ne souhaite pas utiliser de
variables statiques.

Voilà le bouzin modifié qui annule le précédent ( de toute faç on,
il y avait un gros bug dans seuillage.c dû à un copier-coller (un
cast...) ) :


ok. ça correspond beaucoup à ce que j'aurais envie de relire... :-)


Yep, c'est beaucoup plus conventionnel pour un programmeur C averti,
mais je suis sûr qu'il est possible d'améliorer le code. J'entend par
là un optim en terme de temps d'exécution (de la place j'en ai).

A+
Regis


Avatar
Harpo
Targeur fou wrote:


ok. ça correspond beaucoup à ce que j'aurais envie de relire... :-)


Yep, c'est beaucoup plus conventionnel pour un programmeur C averti,
mais je suis sûr qu'il est possible d'améliorer le code. J'entend par
là un optim en terme de temps d'exécution (de la place j'en ai).


Il ne faut pas *trop* voir les choses comme ça, très généralement un bon
code est un code qui tourne et qui est lisible.
L'important, c'est un bon design général et le choix de bons
algorithmes, pas la sophistication du code (j'ai bien dit
*généralement*).

Si le design est bien fait et les algos bien choisis, un code un peu
balourd est correct, on peut toujours finasser sur les détails ensuite
mais cela n'a vraiment d'importance que dans des circonstances assez
précises.
Sans l'avoir disséqué, ton code me parait très correct.

On peut toujours chercher à améliorer son code, c'est très utile car ça
sert à mieux programmer, mais au-delà d'un certain point ça doit être
budgetiser sur la formation.

Juste 2 conseils :

1 ) n'ai pas peur de mettre des commentaires pour chaque variable pour
indiquer ce qu'elle représente. N'hésite pas à charger même un peu.
2 ) inutile de mettre trop de commentaires dans le code, juste une fois
toutes les 15 lignes :
// ici on fait ceci-cela
mais sans plus.
En principe, ça doit être relu par un programmeur. Indique aussi les
trucs u peu Sioux. C'est tout.
3 ) donne le meilleur nom possible au variables, je sais c'est parfois
dificile.

Bon, ben je ne sais pas compter...

--
http://harpo.free.fr/