Discussion :

[oodini]

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.

[r0d]

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.

[oodini]

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

Nous en sommes au même constat.

[r0d]

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 :

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
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...

[Iradrille]

Les noms de fonctions / variables

Bonne chance en tout cas

[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é ).

[oodini]

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.