Conception / passage de this

Hello,

Je travaille sur un projet avec un historique assez lourd.
Je me retrouve avec des classes dotées d'une vingtaine de membres et d'une quarantaine de méthodes. Certaines d'entre elles ont 6 ou 8 arguments.
Pas mal d'entre eux font référence à des données statiques, qui ne bougent plus après leur initialisation, mais qui sont utilisées un peu partout.
Je me trimballe donc souvent des références partagées par moult classes utilisatrices de ces données.

Pour l'instant, j'ai donc des fonctions avec un nombre déraisonnable d'arguments. Une solution est de passer un pointeur vers un objet de la classe appelante, puis d'utiliser des accesseurs. Ou, autre solution, passer un pointeur this aux constructeurs des objets utilisés, qui lui initialisera ses propres références à l'aide de celles déjà présentes dans l'objet appelant.

Cela va me faire une tripotée de références circulaires, heureusement gérables grâce aux déclarations anticipées, mais cela n'est néanmoins pas très propre. Je ne suis pas encore en mesure de revoir l'ensemble de l'archi ; de par la dimension du projet existant, et de la qualité du code présent, je crois que ça va le prendre encore plusieurs mois pour être en mesure d'éventuellement renverser la table.

Auriez-vous des suggestion à me faire pour éviter soit d'avoir de nombreux arguments, soit d'avoir un intercouplage ?

Merci.

Aille, je compatis... ce genre de situation est horrible:
. soit tu commences à revoir l'architecture avec tout les problèmes que ça engendre, et au final, de toutes façons, tu n'auras pas le temps de le faire bien donc au final tu auras quand-même un truc bancale.
. soit tu fais des modifications minimes avec, à chaque fois, le peur que ça te pète au nez.

Je ne pense pas qu'il y ait de solution générale pour ce problème, c'est trop dépendant du contexte. Mais je peux te donner quelques conclusions que j'ai tiré de mes expériences similaires:
- parfois on peut mettre tous les paramètres dans une structure. Et cela a même parfois du sens! Cette structure peut ensuite être un engrenage utile du programme.
- passer le contexte à l'utilisateur (passer this au constructeur d'une classe qu'on utilise) peut être utile dans certains cas, d'ailleurs c'est utilisé dans quelques D.P. Il faut éviter certes, car ça rajoute des dépendances, mais ce n'est pas non plus interdit. Attention à Déméter quand-même.
- même remarque pour passer this en paramètre de fonction. Pas bien mais pas interdit non plus.
- ajouter des couches d'indirection (voir la célèbre citation). Pareil, c'est pas bien mais ça peut résoudre des sacs de nœud à moindre frais. La plupart des D.P. structurels peuvent aider.

OK. Ben merci de m'avoir donné ton avis.

Nous en sommes au même constat. :)

Tiens, juste pour que tu ne te sentes pas seul, je te copie une fonction (oui oui, ce n'est qu'une seule fonction) du code que j'ai récupéré il y a peu et que je dois maintenir.

Code:

---

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 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 342 343 344 345 346 347 348 349 350 351 352 353 354

template <typename TypeDataSource> void DetermineDiscreteAndContinuousOrdinalVariableDistributions(TypeDataSource& dataSource) { size_t memconsumption(GetMemoryConsumptionBinning());     // go through all continuous variables size_t indexEnd(0); while (indexEnd < universeDescription.GetDataDescription().GetCollectionContinuousOrdinal().size()) { ComputeAndShowMemoryInUse();   typedef std::vector<DataDescription::TypeCollectionDescriptionContinuousOrdinalVariable::key_type> TypeCollectionVariablesToTreat;   // QuantileComputationHelper class to encapsulate some variables and functionality for intelligent quantile computation // bpsm: encapulation of auxilary variables into structure. // some of them are free'd here, others aren't. class QuantileComputationHelper { public:   QuantileComputationHelper(size_t numberOfRequiredMemorySlots, const TypeCollectionVariablesToTreat& collectionDescriptionContinuousOrdinalVariable) : quantileMem((float**)malloc(collectionDescriptionContinuousOrdinalVariable.size() * sizeof(float*))) , indexQuantile((int*)malloc(collectionDescriptionContinuousOrdinalVariable.size() * sizeof(int))) { float* tmpq((float*)malloc(numberOfRequiredMemorySlots * sizeof(float)));   if ((tmpq==NULL) || (quantileMem==NULL) || (indexQuantile==NULL)) { std::cout << "out of memory: cannot compute quantiles." << std::endl;   exit(Application::EXIT_ERROR_GENERAL); }   memset(tmpq, 0, numberOfRequiredMemorySlots * sizeof(float)); memset(indexQuantile, 0, collectionDescriptionContinuousOrdinalVariable.size() * sizeof(int));   int i(0); for (TypeCollectionVariablesToTreat::const_iterator it = collectionDescriptionContinuousOrdinalVariable.begin(); it != collectionDescriptionContinuousOrdinalVariable.end(); ++it) { quantileMem[i] = tmpq + 1; tmpq += (*it)->nNonNullInR + 2; // bpsm: +2 for sentinels of intelligent quantiles algorithm indexQuantile[i] = 0;   ++i; } }   ~QuantileComputationHelper() { free(quantileMem[0]-1); free(quantileMem); free(indexQuantile); }       void SetValue(float value, size_t variable) { quantileMem[variable][indexQuantile[variable]] = value; ++(indexQuantile[variable]); }   void FinalizeSetValue() { free(indexQuantile); indexQuantile = NULL; }     float* GetQuantilesMemorySlot(size_t variable) { return quantileMem[variable]; }     private:     float** quantileMem; int* indexQuantile; };         size_t indexBegin(indexEnd);   // Check memory consumption to define how many of the next variables we can read in simultaneously size_t neededSlots(0); while ( (indexEnd < universeDescription.GetDataDescription().GetCollectionContinuousOrdinal().size()) && (neededSlots < memconsumption) ) { DataDescription::TypeCollectionDescriptionContinuousOrdinalVariable::const_iterator it(universeDescription.GetDataDescription().GetCollectionContinuousOrdinal().begin()); std::advance(it, indexEnd);   neededSlots += (*it)->nNonNullInR + 2; // bpsm: +2 for sentinels of intelligent quantiles algorithm   ++indexEnd; }     // collect the variables that need to be treated this time around TypeCollectionVariablesToTreat variablesToTreat;   for (size_t i = indexBegin; i < indexEnd; ++i) { DataDescription::TypeCollectionDescriptionContinuousOrdinalVariable::const_iterator it(universeDescription.GetDataDescription().GetCollectionContinuousOrdinal().begin()); std::advance(it, i);   variablesToTreat.push_back(*it); }     QuantileComputationHelper quantileComputationHelper(neededSlots, variablesToTreat);     // Go through the data source to load values for quantile computation dataSource.Restart(); while (dataSource.GetNext()) { for (size_t i = 0; i < variablesToTreat.size(); ++i) { bool isEmpty(false); double dv = Utility::TextParsing::ReadDoubleFromString(dataSource.GetValue(variablesToTreat[i]->lineIndex), &isEmpty);   // detection of unknown values: if (isEmpty || variablesToTreat[i]->isEmptyValue(dv)) continue;   quantileComputationHelper.SetValue(float(dv), i); } }   quantileComputationHelper.FinalizeSetValue();     std::cout << std::endl;     // Parellel processing: sorting and quantile computation { std::cout << " Sorting and optimal cut computation..." << std::endl;   fflush(0);   float progress(0);   #ifdef _DEBUG // bpsm: in debug there's a complaint of the compiler: // error C3052: '__LINE__Var' : variable doesn't appear in a data-sharing clause under a default(none) clause #pragma omp parallel for shared(variablesToTreat, progress, indexBegin, indexEnd, quantileComputationHelper, std::cout) #else // Fred: removed default(none) #pragma omp parallel for shared(variablesToTreat, progress, indexBegin, indexEnd, quantileComputationHelper, std::cout) #endif for (int currentVariable = 0; currentVariable < static_cast<int>(variablesToTreat.size()); ++currentVariable) { progress = std::max(progress, ((float)((currentVariable+indexBegin+1)*100.0))/static_cast<int>(universeDescription.GetDataDescription().GetCollectionContinuousOrdinal().size()));   printf(" %6.2f %%\r", progress); fflush(0);   #ifdef _DEBUG   if (currentVariable == static_cast<int>(indexBegin)) { std::cout << "number of threads used for quantile computation: " << omp_get_num_threads() << std::endl; }   #endif   float* pQuantilesMemorySlot(quantileComputationHelper.GetQuantilesMemorySlot(currentVariable));         int len = variablesToTreat[currentVariable]->nNonNullInR; std::sort(pQuantilesMemorySlot, pQuantilesMemorySlot + len);       float s=0; for (int j=0; j<len; ++j) s += pQuantilesMemorySlot[j];   variablesToTreat[currentVariable]->mean = s/((double)len); //variablesToTreat[currentVariable]->stdDev = VariableDistribution::stdDeviation(variablesToTreat[currentVariable]->mean, len, pQuantilesMemorySlot); variablesToTreat[currentVariable]->stdDev = Utility::Math::stdDeviation(variablesToTreat[currentVariable]->mean, len, pQuantilesMemorySlot); variablesToTreat[currentVariable]->median = pQuantilesMemorySlot[len/2];   int k = variablesToTreat[currentVariable]->nDiffVal = variablesToTreat[currentVariable]->countModalities(0, len, pQuantilesMemorySlot);   if (k < variablesToTreat[currentVariable]->nMaxQuantile) { // not enough different modalities: fall back to nominal with order case // must compute 'name' (=val) of each modality variablesToTreat[currentVariable]->nModality = k+1; // +1 because of missing variablesToTreat[currentVariable]->nQuantile = k; variablesToTreat[currentVariable]->jumpsv = NULL;   variablesToTreat[currentVariable]->sizeOfMemoryReferredToByPointers = k*sizeof(double)+k*sizeof(float)+(k+1)*sizeof(int);   double* tmpq2 = (double*)malloc(k*sizeof(double)+k*sizeof(float)+(k+1)*sizeof(int)); variablesToTreat[currentVariable]->qv = tmpq2;   float* tmpq3 = (float*)(tmpq2+k); variablesToTreat[currentVariable]->qp = tmpq3;   int* nOcc = (int*)(variablesToTreat[currentVariable]->qp+k); // +1 because of missing variablesToTreat[currentVariable]->nOcc=nOcc;   *nOcc = universeDescription.GetNumberOfLines()-len; // empty/missing   double dvold = (*pQuantilesMemorySlot) - (float)1e30; for (int j=0; j<len; ++j) { double dvnew = (double)*(pQuantilesMemorySlot++); if (dvnew == dvold) ++(*nOcc); else { *(tmpq2++) = dvnew; dvold = dvnew; ++nOcc; *nOcc = 1; *(tmpq3++) = ((float)(j+1))/len; } }   universeDescription.SetNumberOfModalitiesMaximal(std::max(universeDescription.GetNumberOfModalitiesMaximal(), variablesToTreat[currentVariable]->nModality)); } else { int nQuantMod = variablesToTreat[currentVariable]->nModMax; variablesToTreat[currentVariable]->qv = (double*)malloc((2*UniverseDescriptionInfo::MAXIMALNUMBEROFQUANTILES+3*nQuantMod)*sizeof(double)+(nQuantMod+1)*sizeof(int)); // +1 because of missing variablesToTreat[currentVariable]->modv = variablesToTreat[currentVariable]->qv + UniverseDescriptionInfo::MAXIMALNUMBEROFQUANTILES; variablesToTreat[currentVariable]->modv2 = variablesToTreat[currentVariable]->modv + nQuantMod;//NQUANTILESMOD; variablesToTreat[currentVariable]->qp = (float*)(variablesToTreat[currentVariable]->modv2 + nQuantMod); variablesToTreat[currentVariable]->modp = variablesToTreat[currentVariable]->qp + UniverseDescriptionInfo::MAXIMALNUMBEROFQUANTILES;//NQUANTILESMOD; variablesToTreat[currentVariable]->nOcc = (int*)(variablesToTreat[currentVariable]->modp + nQuantMod);//NQUANTILESMOD); variablesToTreat[currentVariable]->jumpsv = NULL; variablesToTreat[currentVariable]->nOcc[0] = universeDescription.GetNumberOfLines()-len; // empty/missing     variablesToTreat[currentVariable]->sizeOfMemoryReferredToByPointers = (2*UniverseDescriptionInfo::MAXIMALNUMBEROFQUANTILES+3*nQuantMod)*sizeof(double)+(nQuantMod+1)*sizeof(int);   size_t tSize(std::max((int)(2*nQuantMod*sizeof(double)), (int)(2*(std::max(UniverseDescriptionInfo::MAXIMALNUMBEROFQUANTILES,nQuantMod)+1)*sizeof(int)))); int* t = (int*)malloc(tSize); memset(t, 0, tSize);   if (!t) { std::cout << "unable to allocate memory to store quantiles." << std::endl; exit(Application::EXIT_ERROR_GENERAL); }   _CrtCheckMemory();   int nCutForcedSz = 0;   cutForced* c = cutArray(UniverseDescriptionInfo::MAXIMALNUMBEROFQUANTILES, &nCutForcedSz, pQuantilesMemorySlot, len, variablesToTreat[currentVariable]->specCut);   _CrtCheckMemory();   k = getCutIdx(c, nCutForcedSz, len, t, UniverseDescriptionInfo::MAXIMALNUMBEROFQUANTILES, pQuantilesMemorySlot, -1) + 1; // +1 bcs nQuantiles=nCuts+1   _CrtCheckMemory();   assert(k <= UniverseDescriptionInfo::MAXIMALNUMBEROFQUANTILES);   //QuantileGraph::Plot(variablesToTreat[currentVariable]->name, pQuantilesMemorySlot, len, t, k-1, c, -1);   free(c); variablesToTreat[currentVariable]->nQuantile=k; for (int j=0; j<k; ++j) { variablesToTreat[currentVariable]->qv[j] = pQuantilesMemorySlot[t[j]]; variablesToTreat[currentVariable]->qp[j] = ((float)t[j]) / len; }   variablesToTreat[currentVariable]->qv[k-1] += (float)1e-10; // to prevent rounding errors   int minBinSz; if (variablesToTreat[currentVariable]->minModSz < 1) minBinSz = (int)(universeDescription.GetNumberOfLines() * variablesToTreat[currentVariable]->minModSz); else minBinSz = (int)variablesToTreat[currentVariable]->minModSz;   minBinSz = std::max(10,minBinSz);   c = cutArray(nQuantMod ,&nCutForcedSz, pQuantilesMemorySlot, len, variablesToTreat[currentVariable]->specCut, minBinSz);   _CrtCheckMemory();   k = getJumpVal(c, nCutForcedSz, len, (float*)t, nQuantMod, pQuantilesMemorySlot, -1) + 1;   _CrtCheckMemory();   variablesToTreat[currentVariable]->nJumps = k; if (k>1) { ((float*)t)[k-1] += (float)1e-10; // to prevent rounding errors variablesToTreat[currentVariable]->jumpsv = (double*)malloc(k*sizeof(double)); variablesToTreat[currentVariable]->sizeOfMemoryReferredToByPointers = k*sizeof(double); for (int j=0; j<k; ++j) variablesToTreat[currentVariable]->jumpsv[j] = (double)((float*)t)[j]; }   k = getCutIdx(c, nCutForcedSz, len, t, nQuantMod, pQuantilesMemorySlot, -1) + 1; // +1 bcs nQuantiles=nCuts+1   _CrtCheckMemory();     if (variablesToTreat[currentVariable]->aGraph) QuantileGraph::Plot(variablesToTreat[currentVariable]->name, pQuantilesMemorySlot, len, t, k-1, c, -1, minBinSz);   free(c); _CrtCheckMemory();     variablesToTreat[currentVariable]->nModality = k+1; // +1 because of missing universeDescription.SetNumberOfModalitiesMaximal(std::max(universeDescription.GetNumberOfModalitiesMaximal(), variablesToTreat[currentVariable]->nModality)); for (int j=0; j<k; ++j) { variablesToTreat[currentVariable]->modv[j] = (double)pQuantilesMemorySlot[t[j]]; variablesToTreat[currentVariable]->modp[j] = ((float)t[j]) / len; variablesToTreat[currentVariable]->modv2[j] = (double)pQuantilesMemorySlot[t[j]+1]; }   variablesToTreat[currentVariable]->modv[k-1] += (double)1e-10; // to prevent rounding errors variablesToTreat[currentVariable]->modv2[k-1] += (double)1e-10; // to prevent rounding errors variablesToTreat[currentVariable]->nOcc[1] = t[0]+1;   for (int j=1; j<k; ++j) { variablesToTreat[currentVariable]->nOcc[j+1] = t[j] - t[j-1]; }   free(t); _CrtCheckMemory(); } }   std::cout << std::endl; } }   // Separate discretized continuous variables from other numerical variables universeDescription.GetDataDescription().SplitContinuousAndDiscretizedVariables(); }

---

Tout le code est de cet acabit. Le problème est différent du tiens, mais c'est le genre du truc qui peut nous tomber sur le coin de la figure de temps en temps...

Les noms de fonctions / variables :aie:

Bonne chance en tout cas 8O

Une déclaration de classe dans une fonction oO. Je ne savais même pas que le compilateur l'acceptait (bizarrement je n'ai jamais essayé :P).

Citation:

---

Envoyé par dragonjoker59

Une déclaration de classe dans une fonction oO. Je ne savais même pas que le compilateur l'acceptait (bizarrement je n'ai jamais essayé :P).

---

La seule limite, c'est pas de classe template dans une fonction ;)

(Pour un foncteur c'est pas trop choquant de déclarer ça dans une fonction, c'est comme une lambda au final, bon dans c'est exemple c'est différent par contre :aie:)

Citation:

---

Envoyé par r0d

Tout le code est de cet acabit. Le problème est différent du tiens, mais c'est le genre du truc qui peut nous tomber sur le coin de la figure de temps en temps...

---

On va tous finir chez les Alcooliques Anonymes. Tel est notre destin.

Citation:

---

Envoyé par dragonjoker59

Une déclaration de classe dans une fonction oO. Je ne savais même pas que le compilateur l'acceptait (bizarrement je n'ai jamais essayé :P).

---

Mieux que ça : dans une boucle. :)

Citation:

---

Envoyé par oodini

Mieux que ça : dans une boucle. :)

---

D'ailleurs je me demande comment c'est compilé cette... heu... chose. J'imagine de le compilo est assez malin pour ne pas redéfinir la classe à chaque itération... enfin j'espère.
Par contre là, dans le code que j'ai posté, en fait ce qui me pose le plus gros problème c'est l'utilisation l'openmp comme ça, brutalement en plein milieu. ça rend le code extrêmement difficile à debugger et impossible à profiler.