abstrakte Basisklasse und Vererbung

bina schrieb:

Wofür wäre denn diese swap methode und wie kann man einen std::vector nachimplementieren??

Das mit dem Vektor vergiss einfach wieder.

Die swap Funktion vereinfacht die Implementierung einiger Funktionen, die noch fehlen. Könnte so aussehen:

#include <algorithm>

void Polynomial::swap(Polynomial& x){
  using std::swap;
  swap(x.pc_, pc_);
  swap(x.n_, n_);
}

Und ja setCoefficient() macht ungefähr das gleiche wie der Konstruktor. Das machen wir uns zu nutze.
Und dann nehmen wir auch noch swap dazu et voilá:

void Polynomial::setCoefficients(const double* const pc, size_t n){
  // bau ein Polynomial und tausch Plaetze mit diesem:
  Polynomial(pc, n).swap(*this);
}

Das war ja einfach! Und alle Widrigkeiten (n!=this->n_) elegant umschifft.

Trotzdem hat Polynomial so wie es jetzt da steht noch ein Riesenproblem:

void f(){
  Polynomial p1;
  Polynomial p2(p1);
} // crash!

Warum macht das Programm hier einen uneleganten Abgang?
Lies mal, über die "Rule of three" in C++.

bina

Das heißt mir fehlen noch copy constructor und copy assignment.

Könnte das nach der deklaration im header dann so aussehen?

Polynomial::Polynomial(const Polynomial& polyn) : pc_(polyn.pc_), n_(polyn.n_)
{
}

Polynomial& Polynomial::operator=(const Polynomial& polyn) {
	pc_ = polyn.pc_;
	n_ = polyn.n_;
	return *this;
}

bina schrieb:

Das heißt mir fehlen noch copy constructor und copy assignment.

Ja. stimmt!

bina schrieb:

Könnte das nach der deklaration im header dann so aussehen?

Polynomial::Polynomial(const Polynomial& polyn) : pc_(polyn.pc_), n_(polyn.n_)
{
}

Polynomial& Polynomial::operator=(const Polynomial& polyn) {
	pc_ = polyn.pc_;
	n_ = polyn.n_;
	return *this;
}

Nein - jetzt hast Du genau die implizit definierten Versionen nachgebaut, die auch der Compiler generiert.
Hast Du denn verstanden, warum die Rule of three wichtig ist, und warum f() oben crasht?

bina

f() crasht, weil der destructor zweimal aufgerufen wird, die selbe adresse zu löschen, oder?

das heißt, ich muss ein neues array erzeugen...

Polynomial::Polynomial(const Polynomial& polyn) {
	n_ = polyn.n_;
	pc_ = new double[polyn.n_];
	for (size_t i = 0; i < polyn.n_; ++i) {
		pc_[i] = polyn.pc_[i];
	}
}

Polynomial& Polynomial::operator=(const Polynomial& polyn) {
	if (&polyn != this) {
		n_ = polyn.n_;
		pc_ = new double[polyn.n_];
		for (size_t i = 0; i < polyn.n_; ++i) {
			pc_[i] = polyn.pc_[i];
		}
	}
	return *this;
}

so in etwa?

Braunstein

prinzipiell schon richtig.
Im Assignment Operator musst du das Array vor dem Neuerzeugen erstmal löschen.
Im Copyconstructor kannst du ruhig auch die Initialisierungsliste nutzen.
Noch schöner ist es den Assignmentoperator über den Copyconstructor zu definieren.
Schau mal nach Copy-Swap-Ideom.

bina schrieb:

f() crasht, weil der destructor zweimal aufgerufen wird, die selbe adresse zu löschen, oder?

das heißt, ich muss ein neues array erzeugen...

Genau. Offensichtlich war das dann ein Flüchtigkeitsfehler mit Deinem ersten Schuß von Copy C'tor und -Assignment.

Versuch die einzelnen Konstruktoren und Operatoren aufeinander aufzubauen. Du kannst u.U. auch C++11 features nutzen, z.B. Konstruktoren bequem aus Konstruktoren aufrufen (Delegating Constructors):

#include <algorithm> // copy

Polynomial::Polynomial(const double* const pc, size_t n)
  : pc_(new double[n]), n_(n)
{
  std::copy(pc, pc+n, pc_);
}

// diser C'tor delegiert an den C'tor oben - der macht dann die Arbeit
Polynomial::Polynomial(const Polynomial& poly)
  : Polynomial(poly.pc_, poly.n_)
{
}

Vergiss auch nicht, dass Du unter anderem auch schon ein swap() im Baukasten hast. Und das von Braunstein erwähnte Copy/Swap Idiom benutzt Du u.U. schon, wenn Du setCoefficients() so übernommen hast, wie sie oben steht.
Du wirst sehen, dass die Zeilenanzahl angenehm übersichtlich bleibt und Du nirgendwo über Sonderfälle stolperst oder manuell Speicher freigeben musst (Ausser natürlich im Destruktor.)

Ich klebe es hier nochmal rein:

void Polynomial::setCoefficients(const double* const pc, size_t n){
  // bau ein Polynomial und tausch Plaetze mit diesem:
  Polynomial(pc, n).swap(*this);
}

Händisch würde man das evtl. so programmieren müssen:

alten Speicher freigeben
neuen Speicher allozieren
n_ kopieren
Daten kopieren

Das alles in nur einer Zeile.

hustbaer

Furble Wurble schrieb:

Händisch würde man das evtl. so programmieren müssen:

alten Speicher freigeben

neuen Speicher allozieren

n_ kopieren

Daten kopieren

Das alles in nur einer Zeile.

Eher
* Neuen Speicher anfordern
* Neuen Speicher initialisieren
* Zeiger- und Grösse-Member auf neuen Speicher verbiegen
* Alten Speicher freigeben

Sonst ist das ganze nämlich nicht Exception-safe.

Marthog

hustbaer schrieb:

Sonst ist das ganze nämlich nicht Exception-safe.

An der exceptionsafety aendert das wenig. Es gibt nur keine strong guarantee.

hustbaer

Marthog schrieb:

hustbaer schrieb:

Sonst ist das ganze nämlich nicht Exception-safe.

An der exceptionsafety aendert das wenig. Es gibt nur keine strong guarantee.

Wenn man nur stur die einzelnen Punkte runtertippt, dann ist es gar nicht exception-safe.
Wenn man es "richtig" macht, und auch der Destruktor passend implementiert ist (wovon man in diesem Fall vermutlich ausgehen kann), dann bekommt man die basic guarantee.

Sogesehen hast du Recht.

Allerdings halte ich die basic guarantee für zu schwach um nützlich zu sein. Wobei es lustigerweise keinen (mir bekannten) Namen für das mMn. nützlichere Mittelding zwischen strong und basic gibt. Nämlich die Garantie dass das Objekt in einem gültigen, aber undefinierten Zustand zurückbleibt. D.h. zusätzlich zur basic guarantee (=nur garantiert dass man das Objekt ohne Probleme löschen kann) noch die Garantie dass man sämtliche Memberfunktionen (und friends bzw. was sonst noch zum Interface der Klasse gehört) aufrufen kann ohne dass es UB gibt.

Und ganz allgemein halte ich es für angebracht bei der Beschreibung von solchen Dingen darauf zu achten nichts wichtiges auszulassen -- auch wenn ein erfahrener Programmierer (der dann allerdings gar keinen Teil der Erklärung braucht, weil er es schon weiss) es sich denken kann.
Also:
* alten Speicher freigeben
*** pc_ auf NULL setzen**
* neuen Speicher allozieren
* n_ kopieren
* Daten kopieren

bina

händisch geschrieben müsste der assignment operator also dann so aussehen:

Polynomial& Polynomial::operator=(const Polynomial& polyn) {
	if (&polyn != this) {
		delete[] pc_;
		pc_ = new double[polyn.n_];
		n_ = polyn.n_;
		for (size_t i = 0; i < polyn.n_; ++i) {
			pc_[i] = polyn.pc_[i];
		}
	}
	return *this;
}

und wenn ich nun das copy/swap ideom benutze, könnte ich das so schreiben?

Polynomial& Polynomial::operator=(const Polynomial& poly) {
	if (&poly != this) {
		Polynomial(poly.pc_, poly.n_).swap(*this);
	}
	return *this;
}

hustbaer

@Marthog
Siehst du jetzt was ich meine?

bina schrieb:

händisch geschrieben müsste der assignment operator also dann so aussehen:

Polynomial& Polynomial::operator=(const Polynomial& polyn) {
	if (&polyn != this) {
		delete[] pc_;
		pc_ = new double[polyn.n_];
		n_ = polyn.n_;
		for (size_t i = 0; i < polyn.n_; ++i) {
			pc_[i] = polyn.pc_[i];
		}
	}
	return *this;
}

Ja.
Du kannst noch an der Reihenfolge drehen z.B. erst wenn eine neue Allozierung erfolgreich war, das alte Array freigeben. (Das wurde von Hustbaer und Marthog angerissen. Exkurs: Stichwort: Exception safety.)
Du kannst auch darauf verzichten neu zu allozieren, wenn Du gleichviel oder weniger Speicher brauchst. Und, und, und...
Aber prinzipiell: ja.

bina schrieb:

und wenn ich nun das copy/swap ideom benutze, könnte ich das so schreiben?
Polynomial& Polynomial::operator=(const Polynomial& poly) {
	if (&poly != this) {
		Polynomial(poly.pc_, poly.n_).swap(*this);
	}
	return *this;
}

Ja.

Da fällt die Wahl doch leicht.

hustbaer

Für copy & swap reicht

Polynomial& Polynomial::operator=(Polynomial other) { 
     swap(other); 
     return *this; 
}

Und für die basic exception-guarantee reicht

delete[] pc_;
        pc_ = 0; // <----------------------
        pc_ = new double[polyn.n_];

volkard

bina schrieb:

und wenn ich nun das copy/swap ideom benutze, könnte ich das so schreiben?
Polynomial& Polynomial::operator=(const Polynomial& poly) {
	if (&poly != this) {
		Polynomial(poly.pc_, poly.n_).swap(*this);
	}
	return *this;
}

Der Test auf Selbstzuweisung ist nicht mehr notwendig.

bina

Vielen vielen Dank euch allen!!!

Eine abschließende Frage hab ich noch. Ich bekomm eine Compilerwarnung und zwar: error C4996: 'std::_Copy_impl': Function call with parameters that may be unsafe...
Ignoriere ich das gekonnt, oder muss ich da was ändern?

bina schrieb:

Ich bekomm eine Compilerwarnung und zwar: error C4996: 'std::_Copy_impl': Function call with parameters that may be unsafe...
Ignoriere ich das gekonnt, oder muss ich da was ändern?

Das ignorierst Du natürlich nicht, sondern behebst es.
Wenn der Fehler nicht offensichtlich ist, kommst Du nicht darum herum uns die fragliche Stelle in Deinem Code zu zeigen.

bina

die fragliche stelle ist in zeile 2132 einer datei namens xutility:

template<class _InIt,
	class _OutIt> inline
	_OutIt copy(_InIt _First, _InIt _Last,
		_OutIt _Dest)
	{	// copy [_First, _Last) to [_Dest, ...)
	if (_First == _Last)
		return (_Dest);
	else
		{	// worth copying, check then copy
		_DEBUG_RANGE(_First, _Last);
		_DEBUG_POINTER(_Dest);
		return (_Copy_impl(_Unchecked(_First), _Unchecked(_Last),
==>			_Dest, _Is_checked(_Dest)));
		}
	}

bina schrieb:

die fragliche stelle ist in zeile 2132 einer datei namens xutility

Wichtig ist die Stelle, wo Du std::copy() aufrufst.

hustbaer

Das kommt immer wenn man "unsafe" Iteratoren verwendet. Wie z.B. Zeiger.
=>
#define _SCL_SECURE_NO_WARNINGS

http://msdn.microsoft.com/en-us/library/aa985974.aspx

bina

Furble Wurble schrieb:

bina schrieb:

die fragliche stelle ist in zeile 2132 einer datei namens xutility

Wichtig ist die Stelle, wo Du std::copy() aufrufst.

Da hab ich deinen Teil übernommen:

Polynomial::Polynomial(const double* const pc, size_t n)
: pc_(new double[n]), n_(n)
{
	std::copy(pc, pc + n, pc_);
}