Discussion :

[bluemartini]

Bonjour,

je suis débutant C++, et je suis perdu malgré les FAQ dans les appels à mes fichiers cpp et hpp...

Voici le schéma de mon programme :
Un fichier index.cpp qui contient le main et qui fait appel à deux classes :

• DrosoGenomicExperiment
• DrosoSeqExperiment

L'entête de main.cpp :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
#include "DrosoGenomic.hpp"
#include "DrosoSeq.hpp"
#include <iostream>

Ma classe DrosoGenomicExperiment définie dans DrosoGenomic.hpp et DrosoGenomic.cpp avec comme entête dans DrosoGenomic.cpp :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
#include "DrosoGenomic.hpp"
#include <fstream>
#include <iostream>
#include <string>
#include <stdio.h>
#include <stdlib.h>

et Ma classe DrosoSeqExperiment définie par les deux fichiers DrosoSeq.hpp et DrosoSeq.cpp avec comme entête pour DrosoSeq.cpp :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
 
#include "DrosoSeq.hpp"
#include "DrosoGenomic.hpp"

La classe DrosoSeqExperiment contient comme attributs deux objets de classe DrosoGenomicExperiment

A la compilation, j'ai des erreurs de mon éditeur depuis que j'ai créé ma classe DrosoSeqExperiment.

Voici les fichiers hpp :

DrosoGenomic.hpp

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
 
#include <stdio.h>
#include <string>
 
using namespace std;  
 
class DrosoGenomicExperiment {
public:
    //constructeur    
    DrosoGenomicExperiment(string label){
        chr2L = new float[23011544];
        chr2R = new float[21146708];
        chr3L = new float[24543557];
        chr3R = new float[27905053];
        chr4 = new float[1351857];
        chrU = new float[10049037];
        chrX = new float[22422827];
        name = label;
    }
    //destructeur
    ~DrosoGenomicExperiment(){
        delete chr2L;
        delete chr2R;
        delete chr3L;
        delete chr4;
        delete chrU;
        delete chrX;
    }
    //méthodes
    string getname(){ return name; }//Give name of object
    bool save(string path);//Save data in a text file. True if it works
    bool restore(string path);//Restore data from a text file. True if it works
    bool load_bed_MGX(string path,int strand);//Load data from a bed file produced by MGX. True if it works
    void extend(int nbnuc);//extend the counts to nbnuc nucleotides, with positive direction
    bool CreateGraphData(string chrom, int start, int stop, string GraphData);//Generate File data for GNUPlot
    void CreateGraph(string chrom, int start, int stop, string GraphFile, string Title);//draw the chromosome chrom from start to stop
 
    //variables
    float *chr2L;//23011544
    float *chr2R;//21146708
    float *chr3L;//24543557
    float *chr3R;//27905053
    float *chr4;//1351857
    float *chrU;//10049037
    float *chrX;//22422827
    string name;
};

et DrosoSeq.hpp :

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
 
#include <stdio.h>
#include <string>
#include <vector>
 
 
using namespace std;  
 
class DrosoSeqExperiment{
    public:
    //constructeur
    DrosoSeqExperiment(string label){
        string labelforward = label + "_forward";
        string labelreverse = label + "_reverse";
        DrosoGenomicExperiment forward = DrosoGenomicExperiment(labelforward);
        DrosoGenomicExperiment reverse = DrosoGenomicExperiment(labelreverse);
        name = label;
    }
    //destructeur
 
    //méthodes
 
    //variables
    string name;
    DrosoGenomicExperiment forward;
    DrosoGenomicExperiment reverse;
};

Merci d'avance pour votre aide

[Médinoc]

Beaucoup de gens ici ont la flemme de recopier plusieurs sources et compiler pour voir les erreurs.

Peux-tu les poster à la place?

[ram-0000]

Médinoc parlait de poster les erreurs bien sûr pas les sources

[koala01]

Salut,

Avant même de vérifier les différents code, quelques remarques qui me sautent aux yeux suite à une lecture en diagonale...

Les fichier d'en-tête <stdio.h> et <stdlib.h> sont des fichiers d'en-tête issus du C...

Il vaut mieux leur préférer (quand on en a réellement besoin) les fichier d'en-tête standard <cstdlib> et <cstdio>

Cependant, du fait que tu inclus le fichier d'en-tête <iostream>, tu n'a même pas besoin de les ajouter (merci l'inclusion en cascade ) (et je ne suis même pas sur que, d'une manière ou d'une autre, l'inclusion du fichier d'en-tête <string> ne finit pas par les inclure en cascade itou... il faudrait vérifier )

De plus, il faut éviter l'utilisation de la directive using namespace dans les fichiers d'en-tête (ce qui implique, malheureusement, qu'il faille envisager l'utilisation de la portée complète (telle que std::string ) dans les fichiers d'en-tête)

La raison est relativement simple: la création d'un espace de noms a pour but de limiter les risques d'ambigüité lorsque tu manipule plusieurs bibliothèques ou lorsque tu crée des structures personnalisées.

Grâce à eux, tu peux envisager sereinement de créer, par exemple, ta propre classe string tout en t'assurant que le compilateur fera la différence avec la classe string qui est fournie par le langage et dont il apprend l'existence par l'inclusion du fichier d'en-tête <string> (fournit par le standard).

Lorsque tu place une directive using namespace (espace de noms), tu dis au compilateur quelque chose qui pourrait s'exprimer sous la forme de

Je sais que j'ai une boite nommée (espace de noms), mais je veux que tu ailles regarder dans cette boite si tu y trouve quelque chose d'intéressant, sans que je n'aie à te le demander explicitement

Et cela a pour conséquence directe de... faire sauter la séparation de ce qui se trouve dans l'espace de noms en question avec... l'espace de noms dans lequel tu est occupé à travailler.

Le problème, si tu place cette directive dans un fichier d'en-tête, c'est que le propre d'un fichier d'en-tête est de pouvoir être inclus dans n'importe quel autre fichier, et que nous remarquons régulièrement un effet d'inclusion en cascade (un fichier d'en-tête qui en inclus un autre, qui en inclus un troisième, un quatrième et un cinquième, qui en incluent eux-même une flopée d'autres, et on peut continuer très longtemps comme cela...), avec comme résultat immédiat que tu perds très facilement le contrôle sur tout ce qui se trouve dans un fichier d'en-tête (dans le sens où tu perd très facilement le contrôle de l'endroit final où se retrouvera son contenu).

Et si un jour, tu viens à vouloir créer ta propre classe string, parce que c'est un terme qui représente de manière correcte l'objet en question (et ça peut ne pas être pour représenter à ta manière une chaine de caractères ), et que, par malheur, le jeu des inclusions en cascade fait que le fichier d'en-tête dans lequel se trouve ta propre classe string se retrouve avec le fichier <string> fourni par le standard, et qu'il y a la directive using namepsace qui apparait du fait de l'inclusion d'un troisième fichier d'en-tête dans lequelle elle apparait, tu va avoir énormément de mal à comprendre que ton compilateur se plaigne de l'ambigüité entre les deux classes string, et surtout, à retrouver l'origine de cette ambigüité...

Alors, je sais qu'il est fatiguant d'utiliser systématiquement std::string, std::cout, std::<y en a tellement, en plus>... mais c'est une habitude qu'il est bon de prendre, au moins dans les fichiers d'en-tête

Ce roman étant écrit, je vais essayer de m'attarder d'avantage sur le code que tu donne

[Noxen]

Je remarque une ou deux choses.

Tu as une classe DrosoGenomicExperiment.
Tu as une classe DrosoSeqExperiment, qui utilises des DrosoGenomicExperiment.

Dans le fichier DrosoSeq.hpp, tu n'inclues pas DrosoGenomic.hpp.
Dans DrosoSeq.cpp, tu inclues DrosoSeq.hpp avant DrosoGenomic.hpp.

Conclusion : au moment où tu compiles DrosoSeq.cpp, le compilo voit qu'il doit aller chercher DrosoSeq.hpp ; dans DrosoSeq.hpp, tu déclares une classe qui a besoin de la classe DrosoGenomicExperiment ; hors celle-ci n'a pas encore été déclarée, puisque tu n'es pas encore entré dans DrosoGenomic => plantage (type non défini).

Donc il te faudrait déclarer dans l'ordre :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
 
#include DrosoGenomic.hpp
#include DrosoSeq.hpp

Sauf que ça ne va pas encore. En fait, il te faut plutôt inclure DrosoGenomic.hpp dans DrosoSeq.hpp, plutôt que dans le .cpp, la raison étant que tu en a besoin dans le .hpp, et pas seulement dans le .cpp. Les problèmes arriveront quand tu voudras inclure DrosoSeq.hpp dans un autre fichier .hpp ou .cpp, et qu'il t'annoncera encore que tu as des types non déclarés. En ne mettant pas DrosoGenomic.hpp dans DrosoSeq.hpp, tu t'imposes d'inclure DrosoGenomic.hpp à chaque fois que tu inclues DrosoSeq.hpp, ce que tu risques d'oublier de faire, et qui est, disons-le, répétitif et c****. DrosoSeq.hpp devrait pouvoir être inclu de manière automnome.


Autre chose :
Je remarque que tu as plusieurs pointeurs de float, qui te servent à désigner des tableaux. Sans revenir sur l'intérêt qu'il y a à utiliser des conteneurs plutôt tableaux bruts, je te suggèrerais de placer ces pointeurs dans un conteneur. std::map, qui permet d'associer une clé à une valeur, pourrait être approprié : tu pourras associer un nom sous la forme d'une std::string à tes tableaux (std::map<std::string, float*> tableaux), ou, si tu veux une vérification à la compilation, les identifiers par une enumération

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
 
enum chr {
CHR2X,
CHR3X,
...
};
 
std::map<chr, float*> tableaux;

L'intérêt est que tu si tu fais des traitements sur tous les tableaux successivement, tu pourras par exemple passer par un itérateur ; et si tu rajoutes un nouveau tableau, tu n'auras pas à retravailler tout ton code pour en tenir compte.

[bluemartini]

Déjà, merci beaucoup pour vos réponses et commentaires!

Voici les erreurs données par le compilateur :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
Type=Error, Priority=Normal, Description=                DrosoGenomicExperiment::DrosoGenomicExperiment(const DrosoGenomicExperiment&)
 
Type=Error, Priority=Normal, Description=                DrosoGenomicExperiment::DrosoGenomicExperiment(const DrosoGenomicExperiment&)
 
Type=Error, Priority=Normal, Description=DrosoGenomicExperiment::DrosoGenomicExperiment(std::string)
 
Type=Error, Priority=Normal, Description=DrosoGenomicExperiment::DrosoGenomicExperiment(std::string)
 
Type=Error, Priority=Normal, Description=no matching function for call to ‘DrosoGenomicExperiment::DrosoGenomicExperiment()’
 
Type=Error, Priority=Normal, Description=no matching function for call to ‘DrosoGenomicExperiment::DrosoGenomicExperiment()’

Pour le choix des tableaux plutôt ques des hash, ces objets sont utilisés pour représentés des chromosomes, et les méthodes à venir vont traiter directement des positions déterminées dans un de ces 'chromosomes', donc je ne voyais pas la valeur ajoutée apportée par un hash.

Je vais prendre du temps pour prendre en compte chacune des remarques.

Merci encore

[Médinoc]

Apparemment, le compilo se plaint que ta classe DrosoGenomicExperiment n'aie pas de constructeur par défaut.

[Noxen]

Pour le choix des tableaux plutôt ques des hash, ces objets sont utilisés pour représentés des chromosomes, et les méthodes à venir vont traiter directement des positions déterminées dans un de ces 'chromosomes', donc je ne voyais pas la valeur ajoutée apportée par un hash.

Ce n'est pas ce que je disais. Au lieu de :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
 
float* chr2L = new float[SIZE];
float* chr2R = new float[SIZE];
float* chr3L = new float[SIZE];
float* chr3R = new float[SIZE];

tu peux faire :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
 
typedef std::map<std::string, float*> MChrom;
MChrom chrom;
chrom["chr2L"] = new float[SIZE];
chrom["chr2R"] = new float[SIZE];
chrom["chr3L"] = new float[SIZE];
chrom["chr3R"] = new float[SIZE];

pour pouvoir faire ensuite, par exemple :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
 
for(MChrom::iterator it = chrom.begin(); it!=chrom.end(); ++it)
    traiter((*it));

plutôt que de devoir écrire le même code 15 fois pour 15 tableaux.

[bluemartini]

Ah ouais ça c'est beaucoup mieux effectivement!!!!!!!
Merci!!!!

[koala01]

Je suis de retour après une lecture un peu plus attentive...

Et cette lecture m'a inspiré quelques réactions supplémentaires (ce n'est encore rien qui puisse te permettre d'avancer dans la résolution de ton problème, mais disons que cela va te permettre de "prendre les bonnes habitudes" )

D'abord, une petite réflexion sur le passage d'arguments à une fonction:

Lorsque tu dois passer un argument à une fonction, et que cet argument est une structure, il est souvent préférable de le passer sous la forme d'une référence (éventuellement constante, si la fonction ne doit pas pouvoir le modifier) à cette fonction pour éviter une copie (qui peut s'avérer très gourmande en temps d'exécution comme en ressources) inutile.

Ainsi, il serait pas mal de passer les std::string sous la forme de référence constante aux constructeur de DrosoGenomicExperiment et aux méthodes (de DrosoGenomicExperiment) save, restore, load_bed_MGX, CreateGraphData et CreateCraph.

De la même manière il serait pas mal que l'accesseur getname renvoie... une référence constante sur le membre name

Au passage, il serait aussi pas mal de définir une politique stricte et cohérente dans la manière de créer tes noms (de variables, de méthodes ou de classes), et de t'y tenir: Le C++ est un langage sensible à la casse (à la différence entre les majuscules et les minuscules), et, à force d'utiliser différentes conventions de nommage des différentes méthodes, tu va finir par ne plus savoir si tu dois appeler la méthode Save, GetName, ou create_graph (or, il se fait qu'aucune des trois n'existe, en l'occurrence )

Je me fous tout à fait de la convention que tu décidera d'appliquer (première lettre de chaque mot en majuscule, séparation de chaque mot par un underscore "_", première lettre du premier mot avec ou sans majuscule,...), mais, si j'ai un conseil à te donner, c'est de décider d'une convention unique, qui sera au moins appliquée à l'ensemble du projet (quitte à ce que tu choisisse une autre convention pour un autre projet)

Ensuite, on peut épingler le fait qu'il est préférable d'éviter les "nombres magiques" (par exemple, toutes ces valeurs que tu utilise dans le constructeur de DrosoGenomicExperiment)...

Tu as surement une raison valable de décider qu'il te faut 23 011 544 de réels pour chr2L ou qu'il t'en faut 21 146 708 pour chr2R, mais ces nombres "sortis de nulle part" ont l'énorme inconvénient de ne donner aucune informations sur leur raison d'être...

Peut être serait il opportun d'envisager de leur donner un nom "représentatif" de leur raison d'être, par exemple (solution à préférer à toute autre) sous la forme de

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
class DrosoGenomicExperiment
{
    public:
        static const int nombreDeChr2L;
        static const int nombreDeChr2R;
    /* suite de la définition de droso.. */
};
/* dans le fichier d'implémentation */
const int DrosoGenomicExperiment::nombreDeChr2L = 23011544;
const int DrosoGenomicExperiment::nombreDeChr2R = 21146708;

Le tout, en adaptant - surement - le nom de ces variables

Cela présentera deux avantages majeurs:

Le premier sera de permettre d'avoir une information sur l'utilité de ce nombre et donc "d'auto commenter" ton code

Le deuxième sera de te permettre, à n'importe quel moment, d'effectuer des tests en prenant cette valeur en référence (par exemple if(i >=DrosoGenomicExperiment::nombreDeChr2L ) ) tout en permettant - si le besoin venait à s'en faire sentir dans le futur - de modifier la valeur sans devoir commencer à reparcourir l'ensemble du code pour aller l'adapter

Tu gagnes donc en facilité de lecture (le simple fait de lire ton code te permet de le comprendre) et en "maintenabilité" de ton code, qui sont deux choses qui entrent pour très grande partie dans la gestion de qualité

Nous pouvons aussi épingler le fait qu'il n'est sans doute pas tout à fait opportun de laisser la chaine de caractères name en accessibilité publique.

en effet, tu dispose d'une méthode qui va se charger de la modifier et d'une autre dont le but est... de la renvoyer.

La première idée qui vient en tête est que... tu veux garder un contrôle "aussi serré que possible" sur cette chaine de caractères, sans doute pour éviter qu'elle ne soit modifiée n'importe comment, sans contrôle, n'importe où dans le code.

Et la conclusion logique de cette décision est... qu'il ne faut pas que l'on puisse y accéder directement en dehors de la classe dont elle fait partie, et donc qu'il est sans doute opportun de rendre ce membre... privé (vraisemblablement) ou protégé (éventuellement, mais ca ne semble pas être le cas ici)

Ensuite, je vais me permettre de revenir un peu sur les différents membre chr*...

Déjà, crois tu opportun de laisser ces membres accessibles sans controle depuis l'extérieur du code

Ne crois tu pas qu'il serait *peut-être* opportun de les protéger des accès extérieurs et de fournir la possibilité (selon les besoin) de les modifier ou de les récupérer au travers d'une méthode qui vérifieront que l'accès est "cohérent"

Faut-il le préciser, je ne suis absolument pas chercheur en biologie, et l'ADN pour moi est un domaine que je ne connais absolument pas (tout au plus puis-je te dire que ca signifie Acide DéhydroNucléique, je crois )

Mais bon, si je ne doute pas que tu aies de bonnes raisons non plus de prévoir un nombre précis et finalement fort important de chacun de ces chr..., je ne peux m'empêcher de me poser la question de savoir s'il est vraiment nécessaire d'en prévoir autant de manière systématique...

Peut-être est-ce le cas, mais il faut avouer que ca en fait quand même "un sérieux paquet"

En plus, je suis sûr d'une chose, c'est que la gestion dynamique de la mémoire est toujours sujette à des risques d'erreur...

La preuve: tu te trompes dans le destructeur de DrosoGenomicExperiment.

Ce n'est pas delete chr2R; qu'il faut écrire, mais, comme tu as demandé l'allocation dynamique d'un tableau de réel, delete[] chr2R (et le raisonnement peut s'appliquer à tous les autres )

Le tout, sans compter que le jour où, par mégarde, tu essayera d'accéder à l'élément se trouvant à un indice supérieur à 23011543 de ton membre chr2R, ton programme partira systématiquement (ou du moins, il aura de grandes chances de partir) en vrille, et le mieux qui puisse alors t'arriver sera un plantage "immédiat" de ton application

Aussi, un des conseils que l'on donne régulièrement sur le forum est de préférer de manière systématique les solutions propres au C++ à toute solution équivalente issue du C.

Et ce conseil s'applique volontiers encore ici: Il se fait que le standard fournit une classe qui permet de sécuriser très fortement l'utilisation des tableaux: la classe vector, disponible dans l'espace de noms std (comme tout ce qui est fourni par le standard) grace à l'inclusion du fichier d'en-tête <vector>.

Que tu décide ou non de prévoir de manière systématique le nombre maximal d'éléments pour chaque membre chr... cette classe te permettra de travailler de manière bien plus sécurisante

La dernière chose sur laquelle je voudrais attirer ton attention est la gestion des différentes responsabilités de tes classes...

Crois tu opportun de déléguer la responsabilité de la création des graphes et sous gnuplot le rendu visuel à ton objet DrosoGenomicExperiment

N'as tu pas l'impression que tu fini par demander vraiment beaucoup de chose à DrosoGenomicExperiment entre la gestion de (centaines de) milliers et même de millions de données différente, la création d'un graphe sous gnuplot et la représentation graphique

Ne serais tu pas un tout petit peu stressé si tu devais t'occuper personnellement (et "a mano") de tout cela en même temps

[EDIT]n'aurais tu pas tendance à mettre un grand écriteau sur ton bureau

L'urgent est parti
l'impossible est en cours

Pour le miracle, nous demandons 24 heures de délais

[bluemartini]

Merci beaucoup pour tous ces précieux conseils!
Effectivement je suis dans l'urgence la plus totale, ce programme est fait pour travailler sur des données générées depuis mardi (date à laquelle j'ai commencé le C++), avec des graphes à produire ...demain!
Tous tes conseils vont être suivis à la lettre, j'y compte bien... Je comptais nettoyer tout ça ce WE!

[bluemartini]

Merci beaucoup pour tous vos précieux conseils! En prenant en compte l'ensemble des remarques, ça marche!
Reste plus qu'à rendre mon code propre à la sauce C++!