C++ verwirrt

C++ verwirrt mich. Folgendes habe ich geschrieben

Matrix* RandomNumberGenerator::multinomialMatrix( double* p, int p_length, int rows, int N_trials ) {

	double sum = 0;
	for( int i = 0; i < p_length; ++i )
		sum += p[i];
	if( sum != 1 )
		return 0;

	Matrix* A = new Matrix( rows, p_length );

	for( int i = 0; i < rows; ++i ) {
		unsigned int* v = getNextMultinomial( p, p_length, N_trials );
		A->addRowVector( v, p_length, i );	  
	}

	A->toString();	
}

unsigned int* RandomNumberGenerator::getNextMultinomial( double* p, int p_length, int N_trials ) {
	unsigned int n[p_length];
	gsl_ran_multinomial( rng, p_length, N_trials, p, n );

	for( int j = 0; j < p_length; ++j )
		cout << n[j] << "\t";
	cout << "\n";

	return n;
}

Die zweite Methode zieht einen Multinomialverteilten Zufallsvektor. Soweit so gut. Das ist die Ausgabe der zweiten Methode:

0 6 3 1
0 3 7 0
1 2 5 2
0 2 5 3
0 2 6 2

Sieht ja ganz gut aus.

Nun habe ich die Ausgabe der zweiten Methode aber in die erste verschoben...

Matrix* RandomNumberGenerator::multinomialMatrix( double* p, int p_length, int rows, int N_trials ) {

	double sum = 0;
	for( int i = 0; i < p_length; ++i )
		sum += p[i];
	if( sum != 1 )
		return 0;

	Matrix* A = new Matrix( rows, p_length );

	for( int i = 0; i < rows; ++i ) {
		unsigned int* v = getNextMultinomial( p, p_length, N_trials );
		for( int j = 0; j < p_length; ++j )
			cout << v[j] << "\t";
		cout << "\n";
		A->addRowVector( v, p_length, i );	  
	}

	A->toString();	
}

unsigned int* RandomNumberGenerator::getNextMultinomial( double* p, int p_length, int N_trials ) {
	unsigned int n[p_length];
	gsl_ran_multinomial( rng, p_length, N_trials, p, n );
	return n;
}

...und erhalte nun verwirrenderweise das hier:

0 4160157172 2693645568 2693645572
0 2693644544 2693645568 2693645572
1 2693644544 2693645568 2693645572
0 2693644544 2693645568 2693645572
0 2693644544 2693645568 2693645572

Was ist denn hier passiert? Ich kann das überhaupt nicht nachvollziehen.

Der Compiler gibt übrengs folgende Warnung aus beim Kompilieren:

g++ -g -c RandomNumberGenerator.cpp -o RandomNumberGenerator.o
RandomNumberGenerator.cpp: In member function ‘unsigned int* RandomNumberGenerator::getNextMultinomial(double*, int, int)’:
RandomNumberGenerator.cpp:50: warning: address of local variable ‘n’ returned

Janjan

Du kannst keine statischen Arrays zurückgeben, wie du es in Zeile 27 tust.

Benutze lieber std::vector oder std::tr1::array.

brotbernd

Naja sagt doch die Fehlermeldung recht deutlich.
unsigned int n[p_length] ist eine lokale Variable, die beim Verlassen des Gültigkeitsbereichs gelöscht wird. Du gibst dann einen Zeiger auf etwas totes zurück. Wenn Du nicht gerade den Umgang mit Zeigern und Speicherverwaltung üben möchtest, solltets Du dir mal die STL ansehen.

Hallo HändyÄndy,

ich schätze mal, dass Du Dich später über das Design ärgern wirst. Da sind so einige Java-ismen drin, die in C++ keine besonders gute Idee sind. ZB dass Du fleißig Dinge per new erzeugst und Zeiger zurück gibst.

Die Lebenszeit des lokalen, automatischen Arrays mit Namen n geht nur bis zum Ende der Funktion. Danach gibt es das Array nicht mehr und ein Zeiger, der auf das erste Element des Arrays zeigte, ist anschließend ungültig.

HändyÄndy schrieb:

unsigned int* RandomNumberGenerator::getNextMultinomial( double* p, int p_length, int N_trials ) {
	unsigned int n[p_length];
	gsl_ran_multinomial( rng, p_length, N_trials, p, n );
	return n;
}

Oh, da ist ja noch mehr falsch. ISO C++ verbietet automatische Arrays, deren Länge nicht zur Compile-Zeit feststeht.

Viel zuviel "Zeigergedöns" hier.

Man könnte es so machen:

std::vector<int> RandomNumberGenerator::getNextMultinomial(
    double const* p, int p_length, int N_trials )
{
    std::vector<int> n (p_length);
    gsl_ran_multinomial( rng, p_length, N_trials, p, &n[0] );
    return n;
}

Du scheinst von Java her zu kommen. Stell Dich darauf ein, dass es noch eine ganze Weile dauert, bis Du "idiomatisches C++" produzieren kannst. Gute Bücher sind auch nicht schlecht. zB "Effective C++" von Scott Meyers.

Lauter gute Tipps, vielen Dank. Ich habe jetzt alles auf Vektoren umgestellt und auf Pointer komplett verzichtet. Ich komme tatsächlich aus der Javawelt... Naja, jetzt muss ich aber C++ machen.

Ich habe aber nun ein neues Problem der Linker meldet folgendes:

g++ -lgsl -o Test Test.o Matrix.o RandomNumberGenerator.o
Undefined symbols:
"Matrix<unsigned int>::addRowVector(std::vector<unsigned int, std::allocator<unsigned int> >, unsigned int)", referenced from:
RandomNumberGenerator::multinomialMatrix(double*, int, int, int)in RandomNumberGenerator.o
"Matrix<unsigned int>::~Matrix()", referenced from:
_main in Test.o
_main in Test.o
RandomNumberGenerator::multinomialMatrix(double*, int, int, int)in RandomNumberGenerator.o
RandomNumberGenerator::multinomialMatrix(double*, int, int, int)in RandomNumberGenerator.o
"Matrix<unsigned int>::toString()", referenced from:
RandomNumberGenerator::multinomialMatrix(double*, int, int, int)in RandomNumberGenerator.o
"Matrix<unsigned int>::Matrix(unsigned int, unsigned int)", referenced from:
RandomNumberGenerator::multinomialMatrix(double*, int, int, int)in RandomNumberGenerator.o
ld: symbol(s) not found
collect2: ld returned 1 exit status
make: *** [Test] Error 1

Versteh ich leider nicht

Janjan

Definitionen von Methoden einer Template-Klasse kommen ebenfalls in die Header-Datei. Du hast die Definitionen sicher in eine Source-Datei ausgelagert -> Falsch.

Danke für die Antwort. Ich habe die Definitionen der Methoden jetzt in die Headerdatei verschoben, allerdings erhalte ich immer noch exakt denselben Fehler.

#ifndef MATRIX_H
#define MATRIX_H	

#include <vector>
#include <iostream>

using namespace std;

template<class T>
class Matrix {

	public:
	std::vector< std::vector<T> > A;
	unsigned int rows,cols;

	Matrix( unsigned int rows_, unsigned int cols_ ) {
		rows = rows_;
		cols = cols_;
		A.resize(rows);
		for( int i = 0; i < rows; ++i )
			A.at(i).resize(cols);
	}

	~Matrix() {}

	void addRowVector( std::vector<T> rowVec, unsigned int rowPos ) {
		if( rows-1 < rowPos )
			return;
		if( rowVec.size() > cols )
			return;

		for( int j = 0; j < cols; ++j ) 		
			A.at(rowPos).at(j) = rowVec.at(j);
	}

	void toString() {

		for( int i = 0; i < rows; ++i ) {
			for( int j = 0; j < cols; ++j )
				cout << A.at(i).at(j) << "\t";
			cout << "\n";
		}
	}

};

#endif

In Matrix.cpp steht nix mehr.

Janjan

Bindest du den Header auch in der Source-Datei ein, in der du die Matrix-Klasse benutzt?

drakon

Das scheint von einer anderen Datei zu kommen.

btw:
Entfern das using namespace noch aus dem Header.

Ja, ich binde die Header-Dateien überall da ein wo sie gebraucht werden.

Z.B. ist die Header-Datei in Test.cpp eingebunden. Der erste Fehler in der Fehlermeldung bezieht sich (meiner Meinung nach) darauf:

#include "Matrix.h"
#include "RandomNumberGenerator.h"
#include <iostream>
#include <stdio.h>

using namespace std;

int main() {
  	RandomNumberGenerator* rng = new RandomNumberGenerator();
  	double p[4] = {0.1, 0.3, 0.5, 0.1 }; 
	Matrix<unsigned int> A = rng->multinomialMatrix( p, 4, 5, 10 ); //4 Zeilen und 5 Spalten

	delete rng;
}

Welcher Fehler? Dass der Linker das eine oder andere Symbol nicht findet? Kann eigentlich nicht sein, wenn Du die Dinger in der Header-Datei definiert hast. Mach dich mal mit der ODR (one definition rule) vertraut. (hint hint)

http://en.wikipedia.org/wiki/One_Definition_Rule
http://fara.cs.uni-potsdam.de/~kaufmann/?page=GenCppFaqs&faq=ODR#Answ

Eventuell musst Du mal all die vom Compiler erzeugten Dateien löschen. Vielleicht benutzt Du unbewusst vorkompilierte Headerdateien, die nicht mehr aktuell sind (?)

HändyÄndy schrieb:

...	
int main() {
    RandomNumberGenerator* rng = new RandomNumberGenerator();
}

Warum new ? Ist doch gar nicht notwendig!

...	
int main() {
    RandomNumberGenerator rng;
}

Versuch demnächst ohne new auszukommen. Es gelingt nicht immer, aber es gelingt oft.

kk

Simon2

krümelkacker schrieb:

...Gute Bücher sind auch nicht schlecht....

Ach!?

Gruß,

Simon2.