Partage
  • Partager sur Facebook
  • Partager sur Twitter

problème classe vecteur template

    3 janvier 2009 à 16:58:27

    Bonjour,

    Alors voilà, j'ai développé une classe vecteur template, que j'utilise pour test dans un petit main avec des entiers et des instances d'une autre classe client. La compilation se passe sans aucun problème.
    Mais lors de l'exécution, déjà avec des entiers, lors de la récupération de la valeur d'indice 3, celle-ci est erronée, pour ce qui est des tests avec la classe client, c'est encore pire, les valeurs sont hasardeuses et le programme fini par planter.
    Je suppose que le problème doit se trouver au niveau des méthodes setElement ou getElement mais je ne vois pas ce qui pose problème.

    Merci de votre aide.

    Vecteur.h
    #ifndef _VECTEUR
    #define _VECTEUR
    #include "Client.h"
    
    template <class T> class Vecteur
    {
    private :
    T *data;
    int *occupation;
    int NbElem;
    
    public :
    Vecteur();
    Vecteur(const int INIT_NbElem);
    ~Vecteur();
    void setElement (int Set_Indice, T Set_Element);
    T getElement(int);
    int NbrElem(void);
    bool estOccupe(int);
    bool indiceValide(int id);
    T retireElement(int pos);
    void affiche();
    T& operator[](int i){return data[i];}
    };
    
    #endif
    


    Vecteur.cpp

    #include "Vecteur.h"
    #include <string>
    #include <iostream>
    #include "Client.h"
    #include <stdlib.h>
    
    using namespace std;
    
    template <class T>
    T Vecteur<T>::getElement(int pos)
    {
         if(occupation[pos]!=0)
              return data[pos];     
    }
    
    template <class T>
    void Vecteur<T>::setElement (int Set_Indice,T Set_Element)
    {
         if(occupation[Set_Indice]==0)
         {
             occupation[Set_Indice] = 1;
             data[Set_Indice] = Set_Element;
         }
    }
    
    template <class T>
    Vecteur<T>::Vecteur ()
    {
        NbElem = 99;
        data=new T[NbElem];
        occupation=new int[NbElem];
        for(int i=0;i<100;i++)
        {
            occupation[i] = 0;        
        }
    }
    
    template <class T>
    Vecteur<T>::Vecteur(const int INIT_NbElem)
    {
        NbElem = INIT_NbElem;       
        data=new T[NbElem];
        occupation=new int[NbElem];
        for(int i=0;i<NbElem;i++)
        {
            occupation[i] = 0;    
        }
    }
    
    template <class T>
    int Vecteur<T>::NbrElem(void){
        int tmp;
        tmp = 0;
        for(int i=0;i<100;i++)
        {
            if(occupation[i]==1)tmp++;        
        }
        return tmp;
     }
    
    template <class T>
    bool Vecteur<T>::estOccupe(int)
    {
        int i = 0;
        bool oqp=false;
       while(i<100)
       {
           if(occupation[i]==0) 
           {oqp=true; break;  }
           i++;        
       }
       return oqp;
    }
    
    template <class T>
    bool Vecteur<T>::indiceValide(int id)
    {
         bool valid=false;
         if (id<=NbElem)
            valid=true;
         else
             valid=false;
         return valid;
    }
    
    template <class T>
    T Vecteur<T>::retireElement(int pos)
    {
             if(occupation[pos]==1)
             {
               occupation[pos] = 0;
               return data[pos];
             }
    }
         
    template <class T>
    Vecteur<T>::~Vecteur ()
    {
       delete [] data;
       delete [] occupation;
    }
    
    template <class T>
    void Vecteur<T>::affiche()
    {
        for (int i=0;i < NbElem;i++)
        {
              cout << data[i] << endl;
        }
    }
    
    template class Vecteur<int>;
    template class Vecteur<Client>;
    


    main.cpp

    #include <stdlib.h>
    #include <iostream.h>
    #include <time.h>
    
    #include "Personne.h"
    #include "Client.h"
    #include "Vecteur.h"
    
    
    int  Menu();
    void Essai1();
    void Essai2();
    
    int main()
    {
      int choix;
      bool fini = false;
      srand((unsigned)time(NULL));
    
      while(!fini)
      {
        choix = Menu();
        switch(choix)
        {
          case 1 : Essai1(); break;
          case 2 : Essai2(); break;
          default : fini = true ; break;
        }
      }
    
      exit(0);
    }
    
    int Menu()
    {
      cout << endl;
      cout << " 1. Test du template Vecteur avec des entiers" << endl; 
      cout << " 2. Test du template Vecteur avec des Clients" << endl;
      cout << " 10. Quitter" << endl << endl;
    
      int ch;
      cout << "  Choix : ";
      cin >> ch;
      return ch;
    }
    
    /*******************************************************************************************************/
    void Essai1()
    {
      cout << "----- 1. Test du template Vecteur avec des entiers ------------------------------------" << endl;
      cout << "Creation  d'un vecteur de 5 cases..." << endl;
      Vecteur<int> vec(5);
      vec.affiche();                                         // --> -- -- -- -- --
      cout << endl;
    
      cout << "L'element d'indice 2 devient 34" << endl;
      if (vec.indiceValide(2)) vec.setElement(2,34);
      vec.affiche();                                         // --> -- -- 34 -- --
      cout << endl;
    
      cout << "L'element d'indice 4 devient 71" << endl;
      if (vec.indiceValide(4)) vec.setElement(4,71);
      vec.affiche();                                         // --> -- -- 34 -- 71
      cout << endl;
    
      cout << "On recupere l'element d'indice 2 (sans le retirer !!!)" << endl;
      if (vec.indiceValide(2) && vec.estOccupe(2))
      { 
        int val= vec.getElement(2);
        cout << "Element recupere = " << val << endl;
      }
      else cout << "Case non occupee ou indice invalide !" << endl;
      cout << endl;
    
      cout << "On recupere l'element d'indice 3 (sans le retirer !!!)" << endl;
      if (vec.indiceValide(3) && vec.estOccupe(3))
      { 
        int val= vec.getElement(3);
        cout << "Element recupere = " << val << endl;
      }
      else cout << "Case non occupee ou indice invalide !" << endl;
      //la ça pose problème, ça devrait retourner 71 mais ça retourne 0!!!!!!
      cout << endl;
    
      vec.affiche();                                         // --> -- -- 34 -- 71
      cout << endl;
    
      cout << "On retire l'element d'indice 2" << endl;
      if (vec.indiceValide(2) && vec.estOccupe(2))
      { 
        int val = vec.retireElement(2);
        cout << "Element recupere = " << val << endl;
      }
      else cout << "Case non occupee ou indice invalide !" << endl;
      cout << endl; 
    
      vec.affiche();                                         // --> -- -- -- -- 71
      cout << endl;
    }
    
    /*******************************************************************************************************/
    void Essai2()
    {
      cout << "----- 2. Test du template Vecteur avec des Clients -------------------" << endl;
      cout << "Creation  d'un vecteur de 7 cases..." << endl;
      Vecteur<Client> vec(7);
      vec.affiche();                 
      cout << endl;
    
      Client client1("Lambrouille","Francois","11/03/1971","Bruxelles","Rue des compilateurs, 21, 4000 Liege","987654321");
      Client client2("Lanflure","Jimmy","05/06/1980","Houtsiplou","Rue des bugs inevitables, 404, 4100 Seraing","111213321");
      cout<<"client1 :"<<endl;
      client1.affiche();
      cout<<"client2 :"<<endl;
      client2.affiche();
      //à partir d'ici ça ne se passe pas comme prévu non plus
      cout << "L'element d'indice 2 devient Lambrouille" << endl;
      if (vec.indiceValide(2)) vec.setElement(2,client1);
      vec.affiche();                                  
      cout << endl;
      
      cout << "L'element d'indice 4 devient Lanflure" << endl;
      if (vec.indiceValide(4)) vec.setElement(4,client2);
      vec.affiche();                                  
      cout << endl;
    
      cout << "On recupere l'element d'indice 2 (sans le retirer !!!)" << endl;
      if (vec.indiceValide(2) && vec.estOccupe(2))
      { 
        Client c = vec.getElement(2);
        cout << "Element recupere = " << c << endl;
        cout << "affiche elem c1 : " << endl;
        c.affiche();
      }
      else cout << "Case non occupee ou indice invalide !" << endl;
      cout << endl;
    
      cout << "On recupere l'element d'indice 3 (sans le retirer !!!)" << endl;
      if (vec.indiceValide(3) && vec.estOccupe(3))
      { 
        Client c = vec.getElement(3);
        cout << "Element recupere = " << c << endl;
      }
      else cout << "Case non occupee ou indice invalide !" << endl;
      cout << endl;
    
      vec.affiche();
      cout << endl;
    
      cout << "On retire l'element d'indice 2" << endl;
      if (vec.indiceValide(2) && vec.estOccupe(2))
      { 
        Client c = vec.retireElement(2);
        cout << "Element recupere = " << c << endl;
      }
      else cout << "Case non occupee ou indice invalide !" << endl;
      cout << endl; 
    
      vec.affiche();   
      cout << endl;
    }
    


    Accessoirement :

    Client.h
    #ifndef CLIENT_H
    #define CLIENT_H
    
    #include "Personne.h"
    
    using namespace std;
    
    class Client : public Personne
    {
          protected:
             char *NumClient;
                    
          public:
             Client();
             Client(char *N, char *P, char *LN, char *DN, char *A, char *NC);
             Client(const Client& C);
             ~Client();
             void setNumClient(char *NC);
             const char *getNumClient() const;
             char *getIdentification();
             Client& operator=(const Client& C);
    	     friend ostream& operator<<(ostream& s, Client& C);
    	     friend istream& operator>>(istream& s, Client& C);
    	     void affiche(void);
    };
    
    #endif
    

    Client.cpp
    #include <stdlib.h>
    #include <iostream>
    #include <string.h>
    #include "Client.h"
    
    using namespace std;
    
    Client::Client()
    {
         cout << "cons defaut cl" << endl;
         setNumClient("123456");
    }
    Client::Client(char *N, char *P, char *LN, char *DN, char *A, char *NC) : Personne(N, P, LN, DN, A)
    {
         cout << "cons init cl" << endl;
         NumClient = new char[strlen(NC)+1];
         strcpy(NumClient, NC);
    }
    Client::Client(const Client& C) : Personne (C)
    {
         cout << "cons copie cl" << endl;
         if (NumClient != NULL) delete [] NumClient;
         NumClient = new char[strlen(C.NumClient)+1];
         strcpy(NumClient, C.NumClient);
    }
    Client::~Client()
    {
         if (NumClient != NULL) delete [] NumClient;
    }
    void Client::setNumClient(char *NC)
    {
         if (NumClient != NULL) delete [] NumClient;
         NumClient = new char[strlen(NC)+1];
         strcpy(NumClient, NC);
    }
    const char *Client::getNumClient() const
    {
          return NumClient;
    }
    char *Client::getIdentification()
    {
         char *tmpid = new char [50];
         strcpy(tmpid,this->getNom());
         strcat(tmpid,this->getNumClient());
         return tmpid;
    }
    
    
    Client& Client::operator=(const Client& C)
    {
         NumClient=C.NumClient;
         return *this;
    }
    ostream& operator<<(ostream& s, Client& C)
    {
         s << C.getNumClient() << endl;
         return s;
    }
    istream& operator>>(istream& s, Client& C)
    {
         char *NC = new char[30];
         cout<<"Numero Client : ";
         s>>NC;
         cout<<endl;
         C.setNumClient(NC);
         return s;
    }
    void Client::affiche(void)
    {
         Personne::affiche();
         cout << "Numero Client : " << NumClient << endl;
         cout << "--------------------------------" << endl;
    }
    
    • Partager sur Facebook
    • Partager sur Twitter
    Anonyme
      3 janvier 2009 à 20:38:46

      Euhhh, déjà petit soucis, comment que tu arrives à compiler sans erreur de link ^^ Car la définition des templates dans un cpp, il n'y a que le compilo borland c++ a ne jamais l'avoir fait (export template)...


      sinon quelque soucis que j'ai spotté:
      • le getelement n'a pas de return si la case n'est pas occupé, mal, à la rigueur, garde une variable static T _Invalid dans le vecteur à retourner dans ce cas là
      • memset(occupation, 0, sizeof(int) * nbElem) pour l'inti c'est plus jolie
      • Conserve un _Count à incrementer/decrementer dans les Set/Clear, car le NbElem à un coup de O(N), il pourait etre O(1)
      • la methode estOccupe prend un int mais ne s'en sert pas, elle a le comportement d'un !empty() <-- TON BUG EST ICI
      • "affiche" affiche meme les cases innocupé

      • Partager sur Facebook
      • Partager sur Twitter
        3 janvier 2009 à 20:57:53

        Je rajouterais qu'il faudrait faire plus attention au const-correctness. Et que l'utilisation d'un allocator serait plus judicieuse à un tableau.
        • Partager sur Facebook
        • Partager sur Twitter
        Anonyme
          3 janvier 2009 à 21:12:10

          Citation : Goten

          Je rajouterais qu'il faudrait faire plus attention au const-correctness. Et que l'utilisation d'un allocator serait plus judicieuse à un tableau.



          Je pousserais plutôt le vice à alloué le block mémoire, et faire des placement new et des appels explicite au destructeur pour eviter de construire tous les objets...

          Passez à l'anglais pour ton code ne serait pas du luxe non plus, de nos jours, coder en francais est une hérésie! Brulons l'hérétique ^^

          Pour une classe aussi "bas niveau", la remplir d'assert comme par exemple dans l'operator[] pour faire un warning en cas de dépassement de range ou si tu accèdes à une case libre
          • Partager sur Facebook
          • Partager sur Twitter
            3 janvier 2009 à 22:03:55

            L'idéal est de délégué la gestion des taches. Une class vector_base qui s'occupe uniquement de l'allocation / désallocation de la mémoire et qui implémenterai le RAII, et une autre classe servant d'interface qui dériverait de la classe de base.


            C'est d'ailleurs ce que fait la class vector de la STL.
            • Partager sur Facebook
            • Partager sur Twitter
              3 janvier 2009 à 22:49:25

              Tout d'abord merci pour votre aide et vos réponses! :)

              Citation : galopin_


              Euhhh, déjà petit soucis, comment que tu arrives à compiler sans erreur de link ^^ Car la définition des templates dans un cpp, il n'y a que le compilo borland c++ a ne jamais l'avoir fait (export template)...



              Euh non aucun soucis au link, j'utilise dev-cpp et ça compile sans problème.

              Il me semble semble avoir déjà vu quelque part que la définition de template dans un cpp ne pose pas de problème à condition d'avoir ces déclarations après :

              template class Vecteur<int>;
              template class Vecteur<Client>;

              Peut-être que je me trompe mais toujours est-il que cela fonctionne.

              Citation : galopin_


              * le getelement n'a pas de return si la case n'est pas occupé, mal, à la rigueur, garde une variable static T _Invalid dans le vecteur à retourner dans ce cas là
              * memset(occupation, 0, sizeof(int) * nbElem) pour l'inti c'est plus jolie
              * Conserve un _Count à incrementer/decrementer dans les Set/Clear, car le NbElem à un coup de O(N), il pourait etre O(1)
              * la methode estOccupe prend un int mais ne s'en sert pas, elle a le comportement d'un !empty() <-- TON BUG EST ICI
              * "affiche" affiche meme les cases innocupé


              • oui, remarque judicieuse je vais corriger ça...
              • memset pour l'inti ? qu'entends-tu par là? l'inti?
              • désolé, je vais peut-être paraître ridicule mais je ne comprend pas bien ta remarque suivante non plus :(
              • pour les deux suivants c'est compris, je vais de ce pas essayer de corriger ça en espérant que ça roulera si d'après toi le bug vient de là :)


              Citation : goten


              Je rajouterais qu'il faudrait faire plus attention au const-correctness. Et que l'utilisation d'un allocator serait plus judicieuse à un tableau.



              Euh, je suppose que tu veux dire la bonne utilisation des const, je sais que je n'ai pas encore vérifié ça. Un allocator? Là non plus je ne suis pas sûr de bien comprendre. Mais il s'agit d'un exercice ou l'on me demande de faire cela, je ne crois donc pas que dans ce cas particulier, l'allocator soit judicieux ;)

              Citation : goten


              L'idéal est de délégué la gestion des taches. Une class vector_base qui s'occupe uniquement de l'allocation / désallocation de la mémoire et qui implémenterai le RAII, et une autre classe servant d'interface qui dériverait de la classe de base.


              C'est d'ailleurs ce que fait la class vector de la STL.



              Je comprend en gros ce que tu veux dire, mais tout cela me semble un peu compliqué, pour l'exercice de base que je dois faire, et pour la suite, je pourrais utiliser la classe vector de la STL, donc je ne vais pas trop compliquer la tâche d'ici là pour rien.


              Encore merci pour votre aide, je fais les corrections et je vous tiens au courant de l'évolution ;)
              • Partager sur Facebook
              • Partager sur Twitter
              Anonyme
                3 janvier 2009 à 22:58:12

                il fallait évidement lire "pour l'init", dsl pour la typo :D

                Et bien quand tu fais AddElement sur une case libre, un petit coup de ++Count;
                Et sur le RemoveElement d'une case qui n'etait pas libre, un petit coup de --Count;

                Pour les templates, cela va varier entre les compilo, tu utilises lequel?
                • Partager sur Facebook
                • Partager sur Twitter
                  3 janvier 2009 à 23:00:53

                  Citation : Pas de titre


                  * désolé, je vais peut-être paraître ridicule mais je ne comprend pas bien ta remarque suivante non plus :(


                  Il parle d'ici :

                  template <class T>
                  int Vecteur<T>::NbrElem(void){
                      int tmp;
                      tmp = 0;
                      for(int i=0;i<100;i++)
                      {
                          if(occupation[i]==1)tmp++;        
                      }
                      return tmp;
                   }
                  


                  En effet tu recalcules à chaque fois. Une méthode plus simple est de faire un ++nbElement (pas sur du nom de ta variable) lors d'un setElement() (qui se comporte comme un add... il aurait été plus judicieux de l'appeler add justement au passage ;) ). Ou une raz lors d'un clear() par exemple ;).

                  Maintenant quelques remarques sur ton code :
                  void n'est pas obligatoire dans les paramétres
                  et int tmp = 0; est plus condensé ^^. Mais c'est un simple détail.
                  Je rajouterais quelque chose : que ce passe-t-il si ton tableau fait plus de 100?


                  Enfin je comprends pas que t'arrive à compiler un template ayant une définition séparé dans un .cpp... :/

                  Citation : Pas de titre

                  Mais il s'agit d'un exercice ou l'on me demande de faire cela


                  On te demande explicitement d'utiliser un tableau ? Si oui alors passe outre nos deux remarques ^^.
                  edit : grilled de peu... ^^.
                  • Partager sur Facebook
                  • Partager sur Twitter
                    4 janvier 2009 à 1:30:47

                    Ta gestion de la chaîne de caractères dans Client est catastrophique. Si l'énoncé de ton exercice te l'autorise, utilise des string, sinon:

                    • A l'initialisation de la classe, NumClient aura une certain valeur, mais probablement pas NULL ce qui va être source de bugs (Par exemple, tu dois avoir 99% de chances de faire un delete sur un pointeur invalide dans ton constructeur par copie ou par défaut). Commence donc toujours par lui affecter la valeur NULL avant de faire quoi que ce soit.
                    • D'ailleurs, dans un constructeur, la classe vient d'être créée, il est donc inutile d'essayer de désallouer quoique ce soit qui aurait pu être créé auparavant.
                    • Par contre, pour ton operateur d'affectation, il est probable que NumClient pointait déjà sur une chaîne auparavant, il faut donc la désallouer. Et il faut faire une copie de la chaîne plutôt que de simplement copier le pointeur si tu ne veux pas avoir de mauvaises surprises quand une des deux instances sera détruite. Bref, tu as fait une fonction setNumClient, bien, mais utilise la.
                    • Faire un delete sur pointeur NULL est correct, ton test est donc inutile.
                    • Partager sur Facebook
                    • Partager sur Twitter

                    problème classe vecteur template

                    × Après avoir cliqué sur "Répondre" vous serez invité à vous connecter pour que votre message soit publié.
                    × Attention, ce sujet est très ancien. Le déterrer n'est pas forcément approprié. Nous te conseillons de créer un nouveau sujet pour poser ta question.
                    • Editeur
                    • Markdown