IdentifiantMot de passe
Loading...
Mot de passe oublié ?Je m'inscris ! (gratuit)
Navigation

Inscrivez-vous gratuitement
pour pouvoir participer, suivre les réponses en temps réel, voter pour les messages, poser vos propres questions et recevoir la newsletter

C Discussion :

Revue de code


Sujet :

C

Vue hybride

Message précédent Message précédent   Message suivant Message suivant
  1. #1
    Rédacteur

    Avatar de Davidbrcz
    Homme Profil pro
    Ing Supaéro - Doctorant ONERA
    Inscrit en
    Juin 2006
    Messages
    2 307
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Âge : 33
    Localisation : Suisse

    Informations professionnelles :
    Activité : Ing Supaéro - Doctorant ONERA

    Informations forums :
    Inscription : Juin 2006
    Messages : 2 307
    Par défaut Revue de code
    Bonjour à tous.

    Je n'ai pas fait de C depuis quelques années et pour des raisons scolaires je dois m'y remettre .

    On me demande dans un exercice de trier un tas de mot selon l'ordre lexicographique.
    La résolution de l'exercice ne me pose pas de soucis en lui même, c'est assez bateau.

    Mais c'est plus sur le point de vue de la propreté du code que j'ai des doutes ainsi que sur les fuites de mémoire et de possible accès hors bornes dans les tableaux (putain de C quoi ....) . Valgrind m'affiche divers choses, mais je ne sais jamais à quoi me fier dans ses rapports car de ce que je m'en rappelle, il lui arrivait de rater le coche sur certains codes.

    Bref, voici le code (un peu long, pas très dur, compile bien avec -Wall et -pedantic)

    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
    42
    43
    44
    45
    46
    47
    48
    49
    50
    51
    52
    53
    54
    55
    56
    57
    58
    59
    60
    61
    62
    63
    64
    65
    66
    67
    68
    69
    70
    71
    72
    73
    74
    75
    76
    77
    78
    79
    80
    81
    82
    83
    84
    85
    86
    87
    88
    89
    90
    91
    92
    93
    94
    95
    96
    97
    98
    99
    100
    101
    102
    103
    104
    105
    106
    107
    108
    109
    110
    111
    112
    113
    114
    115
    116
    117
    118
    119
    120
    121
    122
    123
    124
    125
    126
    127
    128
    129
    130
    131
    132
    133
    134
    135
    136
    137
    138
    139
    140
    141
    142
    143
    144
    145
    146
    147
    148
    149
    150
    151
    152
    153
    154
    155
    156
    157
    158
    159
    160
    161
    162
    163
    164
    165
    166
    167
    168
    169
    170
    171
    172
    173
    174
    175
    176
    177
    178
     
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
     
     
    #define CAPACITY 25
    typedef char** tab2Dchar;
    typedef char* string;
    /*====================================*/
     
    void clean_stdin(void)
    {
        int c;
     
        do {
            c = getchar();
        } while (c != '\n' && c != EOF);
    }
     
    void doubler_taille(string* t,int* n)
    {
     
      string tmp = realloc (*t,
                   2*(*n) * sizeof(*tmp));
          if (tmp != NULL)
          {
             *t = tmp;
         *n=2*(*n);
          }
          else
          {
             /* l'ancien bloc est valide, mais il n'a pas ete agrandi */
          }
    }
     
    /*Vise a recupérer un mot caractère par caractère et à le  renvoyer dans une chaine
    (qui sera au final plus grande que le mot)
    */
     
    string recup_mot()
    {
     
          string tmp=malloc(CAPACITY*sizeof(*tmp));
          char c=getchar();
          int size=CAPACITY;
          int j=0;
     
          while(c!='\n')
          {
            tmp[j]=c;
        j++;
        c=getchar();
     
        /*Si on arrive au bout de tmp à la taille actuelle
          sans être a la fin de l'input, on agrandit tmp*/
        if(j==size-1)
        {
          doubler_taille(&tmp,&size);      
        }
     
          }
          /*Car on veut remplacer  \n par \0 dans tmp*/
          tmp[j]='\0';
          return tmp;
    }
     
    void lecture(tab2Dchar* mots,int n)
    {
     
      int i=0;
      string tmp=NULL;
     
      /*pre affectation pour les n mots à trier*/
      *mots=malloc(n*sizeof(char*));
     
      for(;i<n;++i)
      {
     
          tmp=recup_mot();
          /*A ce stade, tmp contient le i-eme mot mais est trop long*/      
          (*mots)[i]=malloc(strlen(tmp)*sizeof(*tmp));
          strcpy((*mots)[i],tmp);
          free(tmp);
      }
    }
     
    void liberer(tab2Dchar* mots,int n)
    {
        int i=0;
        for (; i < n; ++i)
        {
        free((*mots)[i]);
        }
        free(*mots);
    }
    /*========================================================
    permet d'échanger 2 mots a et b en faisant les allocations/desallocations necessaires
    */
    void swap_mots(string* a,string* b)
    {
        int sizea=strlen(*a);
        int sizeb=strlen(*b);
     
        string tmp=malloc(sizea*sizeof(*a));
        strcpy(tmp,*a);
     
        free(*a);
        *a=malloc(sizeb*sizeof(*b));
        strcpy(*a,*b);
     
        free(*b);
        *b=malloc(sizea*sizeof(*a));
        strcpy(*b,tmp);
     
        free(tmp);
    }
     
    /*
     
    Trouve l'index ou se situe la valeur minimale de tab (tableau de taille n) de tab+a à tab[n-1]
    */
    int min_tab(const tab2Dchar tab,int a,int n)
    {
      int index=a;
      int i=a+1;
      for (; i < n; ++i)
        {
        if(strcmp(tab[i],tab[index])<0)
        {
          index=i;
        }
        }
          return index;
    }
     
    /*
    tri par insertion classique
    */
    void tri_insertion(tab2Dchar tab,int n)
    {
      int i=0;
      int index=0;
      for (; i < n; ++i)
        {
          index=min_tab(tab,i,n);
          swap_mots(&tab[index],&tab[i]);
        }
    }
     
    void afficher(const tab2Dchar mots, int n)
    {
        int i=0;
        for (i = 0; i < n; ++i)
        {
        printf("%s|",mots[i]);
        }
        printf("\n");
    }
    /*=====================================*/
     int main(int argc, char *argv[])
     {
       int n=0;
       tab2Dchar mots=NULL;
     
       printf("Nombre de mots à trier : "); scanf("%d",&n);
       clean_stdin(); /*scanf et fget ne font pas bon ménage, besoin de purger stdin*/
     
       lecture(&mots,n);
     
       afficher(mots,n);
       tri_insertion(mots,n);
       afficher(mots,n);
     
       liberer(&mots,n);
     
      return 0;
    }
    SI vous pouviez me dire ce que vous en pensez. Merci.

    PS/ Je sais que scanf même pour des entiers c'est moyen mais là je vais au plus direct, c'est pas le but de l'exo ....

    Edit / il manquait des const dans le code
    "Never use brute force in fighting an exponential." (Andrei Alexandrescu)

    Mes articles dont Conseils divers sur le C++
    Une très bonne doc sur le C++ (en) Why linux is better (fr)

  2. #2
    Membre très actif
    Homme Profil pro
    Étudiant
    Inscrit en
    Novembre 2010
    Messages
    254
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Localisation : France, Paris (Île de France)

    Informations professionnelles :
    Activité : Étudiant

    Informations forums :
    Inscription : Novembre 2010
    Messages : 254
    Par défaut
    [quote](putain de C quoi ....)|/quote]

    Roh ne maudit pas le C .

    Sinon même si, comme tu l'as dit, ce n'est pas le but de l'exercice, prends l'habitude de ne pas utiliser scanf, car du coup t'es obligé de purger le stdin comme un sagouin.

    Au niveau de ta fonction lecture(), pour envoyer un pointeur sur tableau a deux dimension. Tu pourrais tout simplement prendre en paramètre ton tableau à 2 dimensions et le renvoyer. La du coup t'es obligé de mettre des étoiles et des parenthèses de partout, ce qui est assez moche. Alors je sais que c'est à la mode de faire des fonctions void de partout, mais il n'y a pas de honte a faire des return. Et ça vaut pour toutes les autres fonctions void que tu a fais.

    Après, pour ce qui est du tri par insertion, il est plus facile d'utiliser les listes chainées, car l'insertion d'élément se fait plus facilement.

    Voila tout ce que j'ai à y reprocher.

  3. #3
    Rédacteur

    Avatar de Davidbrcz
    Homme Profil pro
    Ing Supaéro - Doctorant ONERA
    Inscrit en
    Juin 2006
    Messages
    2 307
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Âge : 33
    Localisation : Suisse

    Informations professionnelles :
    Activité : Ing Supaéro - Doctorant ONERA

    Informations forums :
    Inscription : Juin 2006
    Messages : 2 307
    Par défaut
    Pour le coup de la purge, ouais, mais j'avais pas envie de coder une fonction pour choper un entier à partir d'une chaîne.

    Au niveau de ta fonction lecture(), pour envoyer un pointeur sur tableau a deux dimension. Tu pourrais tout simplement prendre en paramètre ton tableau à 2 dimensions et le renvoyer. La du coup t'es obligé de mettre des étoiles et des parenthèses de partout, ce qui est assez moche. Alors je sais que c'est à la mode de faire des fonctions void de partout, mais il n'y a pas de honte a faire des return. Et ça vaut pour toutes les autres fonctions void que tu a fais.
    L'habitude du C++, je vois les fonctions comme des services qui manipulerons (ou créer au besoin) ma liste de mots. Il est vrai que je pourrais renvoyer le tableau crée dans lecture. Mais en faisant ca, je perds une certaine symétrie (allocation/utilisation/destruction) qui me plaît même si je reconnais qu'au final bah passer un char*** bah c'est casse couille et moche. Mais c'est le C qui veut ça en proposant pas un autre mécanisme.....

    Après, pour ce qui est du tri par insertion, il est plus facile d'utiliser les listes chainées, car l'insertion d'élément se fait plus facilement.
    Consigne de l'exo qui obligeait à adapter un code précédent pour trier les mots.
    "Never use brute force in fighting an exponential." (Andrei Alexandrescu)

    Mes articles dont Conseils divers sur le C++
    Une très bonne doc sur le C++ (en) Why linux is better (fr)

  4. #4
    Membre émérite
    Avatar de Elijha
    Homme Profil pro
    Ingénieur développement matériel électronique
    Inscrit en
    Avril 2003
    Messages
    314
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Âge : 55
    Localisation : France, Alpes Maritimes (Provence Alpes Côte d'Azur)

    Informations professionnelles :
    Activité : Ingénieur développement matériel électronique
    Secteur : Bâtiment Travaux Publics

    Informations forums :
    Inscription : Avril 2003
    Messages : 314
    Par défaut
    Bonsoir,

    - Pour la saisie de chaine, tu peu utiliser fgets(...) au lieu de getchar(). La fonction fgets est plus sécuritaire, puisque elle limite au nombre de caractères entrés au clavier (si stdin).
    - Pourquoi pour le swap des mots tu alloues de la mémoire ? il te suffit de permuter les adresses/pointeurs des mots de ton tableau.
    - Conseil: Contrôle tes allocations mémoires (différent de NULL ou pas).

    Je me suis permis une "réécriture" rapide pour l'insertion et le swap
    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
    42
    43
    44
    45
    46
    47
    48
    49
    50
    51
    52
    53
    54
    55
    56
    57
    58
    59
    60
    61
    62
    63
    64
    65
    #include <stdio.h>
    #include <string.h>
     
    char *lecture(void)
    {   char *ptrWord = NULL ;
        char word[128] = "" ;   // On trouve rarement des mots de + de 128 caractères
     
        printf("Mots: ") ;
        if(fgets(word, 128, stdin)!=NULL) {
            // Allouer le bon nombre d'octets de chaine
            if((ptrWord=malloc(strlen(word)))!=NULL) {
                word[strlen(word)-1] = 0 ;          // Supprime le '\n'
                sprintf(ptrWord, "%s", word) ;      // Copie du mot
            }
        }
     
        // Retourne le pointeur (alloué ou pas)
        return ptrWord ;
    }
     
    int main(int argc, char *argv[])
    {   int i, j ;
     
        int nbWord = 0 ;
        char *swapWord = NULL ;
        char **tabWord = NULL ;
     
        // Nombre de mots à trier
        printf("Nombre de mots à trier: ") ;
        if(!scanf("%d", &nbWord)) return EXIT_FAILURE ;
        // Vidage du buffer clavier
        while((i=getchar())!='\n' && (i!=EOF)) ;
     
        // Allocation du tableau
        if((tabWord=malloc(nbWord*sizeof(char*)))==NULL) return EXIT_FAILURE ;
     
        // Lecture des mots et insertion dans le tableau
        for(i=0; i<nbWord; i++) {
            // Insertion
            tabWord[i] = lecture() ;
            // Trie
            for(j=0; j<i; j++)
                // Swap: Permutation des pointeurs de chaines
                if(tabWord[j]!=NULL && tabWord[i]!=NULL && strcmp(tabWord[j], tabWord[i])>0) {
                    swapWord = tabWord[i] ;
                    tabWord[i] = tabWord[j] ;
                    tabWord[j] = swapWord ;
                }
        }
     
        // Affichage des mots triés
        for(i=0; i<nbWord; i++)
            if(tabWord[i]!=NULL)
                printf("%d - %s\n", i, tabWord[i]) ;
     
        // libération des mots
        for(i=0; i<nbWord; i++)
            if(tabWord[i]!=NULL)
                free(tabWord[i]) ;
     
        // Libération du tableau
        free(tabWord) ;
     
        return EXIT_SUCCESS ;
    }

  5. #5
    Rédacteur

    Avatar de Davidbrcz
    Homme Profil pro
    Ing Supaéro - Doctorant ONERA
    Inscrit en
    Juin 2006
    Messages
    2 307
    Détails du profil
    Informations personnelles :
    Sexe : Homme
    Âge : 33
    Localisation : Suisse

    Informations professionnelles :
    Activité : Ing Supaéro - Doctorant ONERA

    Informations forums :
    Inscription : Juin 2006
    Messages : 2 307
    Par défaut
    Salut !

    - Pour la saisie de chaine, tu peu utiliser fgets(...) au lieu de getchar(). La fonction fgets est plus sécuritaire, puisque elle limite au nombre de caractères entrés au clavier (si stdin).
    Je ne peux pas limiter la taille de la chaîne entrée, contrainte de l'exercice. Mon programme doit pouvoir supporter des mots de longueur arbitraire.

    - Pourquoi pour le swap des mots tu alloues de la mémoire ? il te suffit de permuter les adresses/pointeurs des mots de ton tableau.
    Je savais bien que y'avait un truc qui n'allait pas là

    - Conseil: Contrôle tes allocations mémoires (différent de NULL ou pas).
    Encore un réflexe C++. Avec le RAII et les exceptions, ce genre de chose ne se pose plus

    PS/ Tester le résultat de malloc avec un assert, c'est pas trop moche ?
    "Never use brute force in fighting an exponential." (Andrei Alexandrescu)

    Mes articles dont Conseils divers sur le C++
    Une très bonne doc sur le C++ (en) Why linux is better (fr)

  6. #6
    Membre Expert
    Profil pro
    Inscrit en
    Août 2006
    Messages
    1 104
    Détails du profil
    Informations personnelles :
    Localisation : France

    Informations forums :
    Inscription : Août 2006
    Messages : 1 104
    Par défaut
    PS/ Tester le résultat de malloc avec un assert, c'est pas trop moche ?
    Si, ça l'est. A chaque malloc doit être associé un free. Un coup d'assert quelque part dans le code est susceptible de terminer le programme, qui ne peut donc pas libérer (via free) les allocations dynamiques effectuées auparavant avec malloc, s'il y en a.

    assert ou exit, effectués de manière sauvage : même plaie.

    En outre, assert ne fonctionne qu'en mode DEBUG.

Discussions similaires

  1. Outil pour la revue de code
    Par FABFAB125 dans le forum Outils
    Réponses: 7
    Dernier message: 25/11/2007, 10h35
  2. Outils de revue de code
    Par grabriel dans le forum EDI, CMS, Outils, Scripts et API
    Réponses: 2
    Dernier message: 22/08/2007, 11h56
  3. Outils de revue de code
    Par YAMKI dans le forum Qualimétrie
    Réponses: 2
    Dernier message: 15/02/2006, 12h29
  4. [Conseil] revue de code
    Par allstar dans le forum Langage
    Réponses: 2
    Dernier message: 09/11/2005, 11h02
  5. [Revue de code] Quels outils pour de grosses applis?
    Par franckR dans le forum Choisir un environnement de développement
    Réponses: 1
    Dernier message: 21/03/2004, 10h03

Partager

Partager
  • Envoyer la discussion sur Viadeo
  • Envoyer la discussion sur Twitter
  • Envoyer la discussion sur Google
  • Envoyer la discussion sur Facebook
  • Envoyer la discussion sur Digg
  • Envoyer la discussion sur Delicious
  • Envoyer la discussion sur MySpace
  • Envoyer la discussion sur Yahoo