STL vector vs. array of pointers

[Kellerautomat]

Wie dir schon mehrmals gesagt wurde, ist das Problem nicht der std::vector (der kopieren oder moven muss, wenn realloziert wird), sondern dass dein Dummy keine korrekten Semantiken implementiert. std::vector hat Anforderungen an den Typen, von dem du Objekte speichern moechtest. Diese kannst du auf http://en.cppreference.com/w/cpp/container/vector unter "Template parameters" nachlesen. Dort steht:

The requirements that are imposed on the elements depend on the actual operations performed on the container. Generally, it is required that element type meets the requirements of MoveConstructible and MoveAssignable, but many member functions impose stricter requirements. (since C++11)

Da ich davon ausgehe, dass dir das jetzt nicht besonders weiterhilft, wuerde ich einen fertigen C++ Wrapper verwenden, der dir die korrekten Semantiken bereits anbietet. Dann hast du naemlich am wenigsten Arbeit und einen von vielen Leuten getesteten Wrapper, der wahrscheinlicher ( ) korrekt ist.

Ansonsten sehe ich noch 2 andere Moeglichkeiten: Eine schoene, und eine schnelle. Im folgenden werde ich die GSL Resourcen nur schematisch andeuten, ich habe mich mit dieser Bibliothek nicht naeher beschaeftigt.

a) Die schnelle (im Sinne von "schnell umgesetzt", nicht "performant") Variante: Du verbietest Kopien bei deinem Dummy voellig und reichst Heap-allozierte Dummy-Objekte per Smartpointer herum. Das ist im Prinzip genau das, was du gerade versuchst zu machen, nur in richtig. :p

struct Dummy
{
    Dummy(...) : stuff_(gsl_alloc_stuff(...)) {}
    ~Dummy() { gsl_free_stuff(stuff_); }

    void do_something() { gsl_something(stuff_); }

    // Kopien verbieten
    Dummy(Dummy const&) = delete;
    Dummy& operator = (Dummy const&) = delete;

private:
    gsl_stuff* stuff_;
};

-->

std::vector<std::unique_ptr<Dummy>> dummies;
dummies.emplace_back(new Dummy);
dummies[0]->do_something();

b) Die schoene Variante: Du baust dir einen korrekten Wrapper. Hierzu musst du erst einmal das Konzept einer Resource verstehen. Was ist also eine Resource? Eine Resource hat ueblicherweise genau einen Besitzer, der sie dann freigibt, wenn er sie nicht mehr braucht. Das haben wir bei der "schnellen Variante" mit dem std::unique_ptr sichergestellt. unique_ptr macht naemlich genau das: Er ist nicht kopierbar, aber er erlaubt es, den Besitz der Resource an einen anderen unique_ptr zu uebergeben. Dieses Uebergeben des Besitzes nennen wir im modernen C++11 Jargon "move" oder "Move Semantiken". Fuer Move-Semantiken brauchen wir allerdings einen "Leer-Zustand", in deinem Fall koennte das ein Nullpointer sein.

Das schreibt man in Code dann so hin:

struct Dummy
{
    // Default-Konstruktor: "leer"-Zustand
    Dummy()
        : stuff_(nullptr)
    {}

    // "normaler" Konstruktor mit Argumenten
    Dummy(...)
        : stuff_(gsl_alloc_stuff(...))
    {}

    // Move-Konstruktor: Resource von 'other' klauen und 'other' in den "leer"-Zustand setzen
    Dummy(Dummy&& other)
        : stuff_(other.stuff_)
    {
        other.stuff_ = nullptr;
    }

    // Move-Assignment: *this zerstoeren, danach wie Move-Konstruktor
    Dummy& operator = (Dummy&& other)
    {
        if(stuff_)
            gsl_free_stuff(stuff_);

        stuff_ = other.stuff_;
        other.stuff_ = nullptr;

        return *this;
    }

    // Destruktor: Leerzustand beachten, falls gsl_free_stuff nicht mit Nullzeigern umgehen kann
    ~Dummy()
    {
        if(stuff_)
            gsl_free_stuff(stuff_);
    }

private:
    gsl_stuff* stuff_;
};

Diese Klasse kannst du dann direkt in einem std::vector verwenden:

std::vector<Dummy> dummies;
Dummy d(...);
dummies.push_back(std::move(d)); // das herumschieben der Resourcen muss explizit mittels std::move geschehen

Wie allerdings schon im anderen Thread gesagt wurde, ist ein fertiger Wrapper ueblicherweise zu bevorzugen.

Der Kellerautomat

[Ethon]

Da dich eh gleich jemand darauf hinweisen würde ...

// Move-Assignment: *this zerstoeren, danach wie Move-Konstruktor
    Dummy& operator = (Dummy&& other)
    {
        if(stuff_)
            gsl_free_stuff(stuff_);

        stuff_ = other.stuff_;
        other.stuff_ = nullptr;

        return *this;
    }

Hier fehlt ein

if(this == &other) return *this;

[Klaus82]

Hallo Kellerautomat,

vielen Dank für deine ausführliche Antwort und das Beispiel.

Ich habe das versucht umzusetzen (siehe unten) auf mein obiges Beispiel und ich bin platt: Es funzt!!

• Im nächsten Schritt werde ich versuchen die Methode auf die Verwendung der GSL zu übertragen.
• Natürlich hast du recht, dass es besser ist einen getesteten Wrapper zu verwenden. Aber ich habe den Eindruck, dass ein Wrapper genau das tut, was ich gerade lerne. Und so lange ich das nicht richtig verstehe ist es sicherlich ebenso fahrlässig einen Wrapper einzusetzen.
• Leider erlaubt mein gcc compiler keine Verwendung von nullptr , da dies laut google erst ab gcc 4.6 möglich ist, ich unter Debian Squeeze aber gcc 4.4.5 habe. Ich hoffe eine '0' einzusetzen ist auch okay.

Ethon schrieb:

Hier fehlt ein

if(this == &other) return *this;

Wie meinen? Wenn ich die if-Abfrage einbaue, dann habe ich sowohl in der if-Abfrage als auch 'normal' ein return *this .
Wenn ich die if-Abfrage anstatt des return statements einbaue, dann kann es sein, dass ich nichts zurückgebe, falls die if-Abfrage false liefert?

Viele Grüße,
-- Klaus.

// dummy.cpp
#include <iostream>
#include <vector>
using std::cout;
using std::endl;
using std::vector;
using std::move;

struct dummy
{
    dummy() : val(42) {}
    unsigned int val;
};

struct foo
{
	// Konstruktor
	foo() : d_(new dummy) {}
	// Move-Konstruktor 
	foo(foo&& other) : d_(other.d_) { other.d_ = 0; }
	// Move-Zuweisungsoperator
	foo& operator=(foo&& other)
	{
		if(d_)
			delete d_;

		d_ = other.d_;
		other.d_ = 0;

		return *this;
	}
	// Destruktor
  ~foo()
	{
		if(d_)
			delete d_;
	}

  struct dummy* d_;
};

int main()
{
	vector<struct foo> foos;

	foos.push_back(move(foo()));

	cout << foos[0].d_->val << endl;

  return 0;
}

[Nexus]

Klaus82 schrieb:

Tada!! Fertig. Kein Speicherzugriffsfehler, kein Memory-Leak, kein gar nichts.

Gib mal ein bar -Objekt aus einer Funktion zurück, dann reden wir weiter. Dein Design ist eine tickende Zeitbombe.

Ethon schrieb:

Da dich eh gleich jemand darauf hinweisen würde

Oder man macht es direkt mit Copy-and-Swap...

[Nexus]

using std::cout;
using std::endl;
using std::vector;
using std::move;
// Würde ich nicht kategorisch machen, das hebelt den Sinn von  Namensräumen aus 
// und macht schlechter erkennbar, dass es sich um die Standardbibliothek handelt

foo(foo&& other) : d_(other.d_) { other.d_ = 0; }
// Nimm nullptr für Nullzeiger-Konstanten

		if(d_)
			delete d_;
// delete auf Nullzeiger ist erlaubt (kein Effekt)

		d_ = other.d_;
		other.d_ = 0;

		return *this;
// schau dir mal Copy-and-Swap an. Eine swap()-Funktion kann auch sonst nützlich
// sein, und damit wird die Implementierung trivial.

struct dummy* d_;
vector<struct foo> foos;
// struct bei der Variablendeklaration ist in C++ unnötig und unüblich.

foos.push_back(move(foo()));
// Das std::move() ist unnötig, du hast bereits einen RValue.
// Lies dich in Move-Semantik ein, z.B. die Artikelserie auf C++Next
// http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/

Aber du baust dir mit foo ohnehin nur einen limitierten std::unique_ptr nach, dann kannst du gleich letzteren verwenden. Kellerautomats Hinweis war für andere Ressourcen gedacht, wo nicht nur Speicher verwaltet werden muss.

[Swordfish]

...

[Klaus82]

Nexus schrieb:

foo(foo&& other) : d_(other.d_) { other.d_ = 0; }
// Nimm nullptr für Nullzeiger-Konstanten

		if(d_)
			delete d_;
// delete auf Nullzeiger ist erlaubt (kein Effekt)

		d_ = other.d_;
		other.d_ = 0;

		return *this;

Siehe meine Aussage dazu oben:

Klaus82 schrieb:

• Leider erlaubt mein gcc compiler keine Verwendung von nullptr , da dies laut google erst ab gcc 4.6 möglich ist, ich unter Debian Squeeze aber gcc 4.4.5 habe. Ich hoffe eine '0' einzusetzen ist auch okay.

Nexus schrieb:

Aber du baust dir mit foo ohnehin nur einen limitierten std::unique_ptr nach, dann kannst du gleich letzteren verwenden. Kellerautomats Hinweis war für andere Ressourcen gedacht, wo nicht nur Speicher verwaltet werden muss.

Ich muss das alles erstmal verdauen. Ich bin fern davon zu behaupten, ich hätte was kapiert, sondern bestenfalls verstanden was das Problem ist: Dass ich den Speicherbereich eines Pointers nicht zwei Mal freigeben kann, im Objekt und dessen Kopie, leuchtet mir ein.

Die ganzen Details und cool Features mit den vielen Schlagwörten, die in diesem und dem anderen Thread gefallen sind muss ich mir nach und nach erarbeiten.

Jetzt funzt es auch mit der GSL, siehe unten!

Viele Grüße und noch ein schönen Restpfingstmontag,
-- Klaus.

// mb.cpp
#include <iostream>
#include <vector>
#include <gsl/gsl_errno.h>
#include <gsl/gsl_spline.h>

using namespace std;

struct foo
{
void build (unsigned int const);
void clean_up();
vector<double> x_values;
vector<double> y_values;
};

void foo::build(unsigned int const m)
{
	for(unsigned int i = 0; i < 100; ++i)
	{
		x_values.push_back(i);
		y_values.push_back(m*i);
	}
}

void foo::clean_up()
{
	x_values.clear();
	y_values.clear();
}

struct bar
{
	// Default Konstruktor
	bar() : acc(0), spline(0) {}
	// Konstruktor
	bar(std::vector<double> &,std::vector<double> &);
	// Move-Konstruktor
	bar(bar&& other) : acc(other.acc), spline(other.spline)
	{
		other.acc = 0;
		other.spline = 0;
	}
	// Move-Zuweisungsoperator
	bar& operator=(bar&& other)
	{
		if(acc)
			gsl_interp_accel_free(acc);
		if(spline)
			gsl_spline_free(spline);

		acc = other.acc;
		spline = other.spline;

		other.acc = 0;
		other.spline = 0;

		return *this;
	}
	~bar();
	vector<double> x_values;
	vector<double> y_values;
	double get(double);

	gsl_interp_accel* acc;
	gsl_spline* spline;
};

bar::bar(std::vector<double> & x, std::vector<double> & y):
x_values(x), y_values(y),
acc(gsl_interp_accel_alloc()),spline(gsl_spline_alloc(gsl_interp_cspline,x_values.size()))
{
	gsl_spline_init(spline,& x_values.front(),& y_values.front(),x_values.size());
}

bar::~bar()
{
	gsl_interp_accel_free(acc);
	gsl_spline_free(spline);
}

double bar::get(double x)
{
	return gsl_spline_eval(spline,x,acc);
}

int main()
{
	vector<bar> bars;
	foo f;

	f.build(1);

	bars.push_back(move(bar(f.x_values,f.y_values)));

	f.clean_up();

	f.build(2);

	bars.push_back(move(bar(f.x_values,f.y_values)));

	f.clean_up();

	cout << bars[0].get(3.5) << endl;
	cout << bars[1].get(3.5) << endl;

	return 0;
}

[Klaus82]

Hallo,

ich habe dazu mal wieder eine Frage:

Wenn ich ganz am Ende des obigen Codes noch eine weitere Zeile einfüge, weil ich z.B. eine Ausgabe haben möchte:

cout << bars[0].x_values.size() << endl;

Dann kriege ich als Ergebnis eine satte Null!

Aber wie kann das sein? Wenn der Vektor der x- und y-Werte leer wäre, dann könnte die GSL doch gar keinen Wert interpolieren?
Oder hängt das damit zusammen, dass ich zwar im Konstruktor mittels x_values und y_values den Spline für die Interpolation erstelle, aber dann nur noch diesen mittels move-Konstruktor weitergebe?

Gruß,
-- Klaus.

[Furble Wurble]

Klaus82 schrieb:

Oder hängt das damit zusammen, dass ich zwar im Konstruktor mittels x_values und y_values den Spline für die Interpolation erstelle, aber dann nur noch diesen mittels move-Konstruktor weitergebe?

Genauso hast Du es doch geschrieben. Die Daten kommen in der Folge nur noch aus dem gsl_spline Objekt.
Ich verstehe die Frage nicht.

[Klaus82]

Hallo Furble Wurble,

Furble Wurble schrieb:

Klaus82 schrieb:

Oder hängt das damit zusammen, dass ich zwar im Konstruktor mittels x_values und y_values den Spline für die Interpolation erstelle, aber dann nur noch diesen mittels move-Konstruktor weitergebe?

Genauso hast Du es doch geschrieben. Die Daten kommen in der Folge nur noch aus dem gsl_spline Objekt.
Ich verstehe die Frage nicht.

Ja, aber jetzt würde ich gerne noch den Fall einbauen, dass der zu interpolierende Wert nicht in dem Bereich liegt, den ich zur Interpolation vorgebe:

double bar::get(double x)
{
    if(x <= x_values.front() || x >= x_values.back())
      return 0.0;

    return gsl_spline_eval(spline,x,acc);
}

Nur wenn dann x_values nicht mehr existiert, dann habe ich ein Problem.

EDIT:
Also ich habe den default konstruktor, konstruktor und move konstruktor um folgende Zeilen ergänzt: Jetzt scheint es zu funktionieren:

double const x_min;
double const x_max;

/*
---{}--- default constructor ---{}---
*/
x_min(0),x_max(0)

/*
---{}--- constructor ---{}---
*/
x_min(x_values.front()),x_max(x_values.back())

/*
---{}--- move constructor ---{}---
*/
x_min(other.x_min),x_max(other.x_max)

Gruß,
-- Klaus.

[Furble Wurble]

Du kommst an die Daten natürlich auch über das gsl_spline Objekt:

double bar::get(double x)
{
    if(x <= spline->x[0] || spline->x[spline->size-1] <= x)
      return 0.0;

    return gsl_spline_eval(spline,x,acc);
}

Schön ist was anderes...
Das mit den Vektoren, die einfach fallen gelassen werden beim moven ist auch Mist. Grübel nochmal über Dein Design.
Zumindest sollte immer gewährleistet sein, dass das gsl_spline Objekt und das bar Objekt Ihre Daten synchron halten - besser noch es gibt nur eine Kopie der Daten.

Dein Edit habe ich mir nicht angesehen - scheint aber auch nicht an meinen Kritikpunkten anzusetzen.