C++ ... unbeholfen bei der Objektorientierung. Fehler beim Client.

[Jolka]

Hallo,
ich bin blutiger Anfänger bei C++. Ich komme eigentlich aus der Java-Sparte. C habe ich mir in den letzten 2 Monaten auch angeeignet. Jetzt versuche ich mich daran was objektorientiertes mit C++ auf die Beine zu stellen, doch in der main-Methode bekomme ich immer folgende Fehler, die ich mir nicht erklären kann und schon seit Ewigkeiten sitze ich daran fest.

Line 13: error: request for member 'add' in 'cart', which is of non-classtype 'ShoppingCart*'
Line 14: error: request for member 'add' in 'cart', which is of non-classtype 'ShoppingCart*'
Line 15: error: request for member 'add' in 'cart', which is of non-classtype 'ShoppingCart*'
Line 16: error: request for member 'add' in 'cart', which is of non-classtype 'ShoppingCart*'
Line 17: error: request for member 'getTotalCost' in 'cart', which is of non-classtype 'ShoppingCart*'

Zum Programm:
CartItem beschreibt ein Produkt mit Name, Anzahl und Einzelpreis.
ShoppinCart ist der Einkaufswagen, der CartItem-Objekte in einer vector-Liste enthält.
Die main-Methode soll nun einfach ein paar Produkte in den Einkaufswagen legen und den Gesamtpreis ausgeben.

Wie gesagt ich hänge hier schon seit Ewigkeiten fest und ich versteh nicht was ich da falsch mache. Der Vollständigkeit halber einfach mal der gesamte Quelltext, falls ich irgendwas falsch deklariert hab oder nicht richtig initialisiert oder was auch immer. Bei C++ bin ich noch sehr unbeholfen was das angeht.

CartItem.h

#ifndef CARTITEM_H
#define CARTITEM_H
#include <string>
class CartItem {
    private:
        std::string name;
        int quantity;
        double pricePerUnit;

    public:
        CartItem(const std::string name,const int quant,const double price);
        double getCost();
        std::string getName();
        int getQuantity();
        double getPricePerUnit();
        void setQuantity(const int n);
        void setPricePerUnit(const double price);
        void setName(const std::string name);
};
#endif

CartItem.cpp

#include "CartItem.h"

CartItem::CartItem(const std::string name, const int quant, const double price) {
    setName(name);
    setQuantity(quant);
    setPricePerUnit(price);
}

double CartItem::getCost() {
    return quantity*pricePerUnit;
}

std::string CartItem::getName() {
    return name;
}

int CartItem::getQuantity() {
    return quantity;
}

double CartItem::getPricePerUnit() {
    return pricePerUnit;
}

void CartItem::setQuantity(const int n) {
    quantity = n;
}

void CartItem::setPricePerUnit(const double price) {
    pricePerUnit = price;
}

void CartItem::setName(const std::string name) {
    this->name = name;
}

ShoppingCart.h

#ifndef SHOPPINGCART_H
#define SHOPPINGCART_H
#include <vector>
#include "CartItem.h"
class ShoppingCart {
    private:
        std::vector<CartItem> contentList;
    public:
        ShoppingCart();
        void add(const CartItem item);
        double getTotalCost();
        int getNumberOfItems();
        CartItem getItem(const int i);
};
#endif

ShoppingCart.cpp

#include "ShoppingCart.h"

/*ShoppingCart::ShoppingCart() {

}*/

void ShoppingCart::add(const CartItem item) {
    contentList.push_back(item);
}

double ShoppingCart::getTotalCost() {
    double totalCost=0;
    for(unsigned int i=0; i<contentList.size(); i++) {
        totalCost += contentList.at(i).getCost();
    }
    return totalCost;
}

int ShoppingCart::getNumberOfItems() {
    return contentList.size();
}

CartItem ShoppingCart::getItem(const int i) {
    return contentList.at(i);
}

main.cpp

#include <iostream>
#include "ShoppingCart.h"
#include "CartItem.h"

using namespace std;

int main() {
    ShoppingCart* cart = new ShoppingCart();
    CartItem* hundefutter = new CartItem("Hundefutter", 6, 3.20);
    CartItem* kekse = new CartItem("Kekse", 4, 1.59);
    CartItem* milch = new CartItem("Milch", 1, 0.69);
    CartItem* marmelade = new CartItem("Erdbeerkonfituere", 3, 2.19);
    cart.add(hundefutter);
    cart.add(kekse);
    cart.add(milch);
    cart.add(marmelade);
    cout << cart.getTotalCost();
}

[Gast]

Cart ist ein Zeiger. Nim den Pfeiloperator.

#include <iostream>
#include "ShoppingCart.h"
#include "CartItem.h"

using namespace std;

int main() {
    ShoppingCart* cart = new ShoppingCart();
    CartItem* hundefutter = new CartItem("Hundefutter", 6, 3.20);
    CartItem* kekse = new CartItem("Kekse", 4, 1.59);
    CartItem* milch = new CartItem("Milch", 1, 0.69);
    CartItem* marmelade = new CartItem("Erdbeerkonfituere", 3, 2.19);
    cart->add(hundefutter);
    cart->add(kekse);
    cart->add(milch);
    cart->add(marmelade);
    cout << cart->getTotalCost();
    delete cart;
    delete hundefutter;
    delete kekse;
    delete milch;
    delete marmelade;
}

Und vergiss nicht, den Speicher freizugeben! Keine Garbage-Collection!! :xmas1:

[SeppJ]

Hacker schrieb:

Cart ist ein Zeiger. Nim den Pfeiloperator.

...

Und vergiss nicht, den Speicher freizugeben! Keine Garbage-Collection!! :xmas1:

Nein. Mach das stattdessen gar nicht erst so. Du machst Java in C++. Wenn du new in C++ benutzt ohne einen besonderen Grund, dann machst du etwas falsch.

int main() {
    ShoppingCart cart;
    CartItem hundefutter("Hundefutter", 6, 3.20);
    CartItem kekse("Kekse", 4, 1.59);
    CartItem milch("Milch", 1, 0.69);
    CartItem marmelade("Erdbeerkonfituere", 3, 2.19);
    cart.add(hundefutter);
    cart.add(kekse);
    cart.add(milch);
    cart.add(marmelade);
    cout << cart.getTotalCost();
}

Oder unnötige Zwischenwerte:

int main() {
    ShoppingCart cart;
    cart.add(CartItem("Hundefutter", 6, 3.20));
    cart.add(CartItem("Kekse", 4, 1.59));
    cart.add(CartItem("Milch", 1, 0.69));
    cart.add(CartItem("Erdbeerkonfituere", 3, 2.19));
    cout << cart.getTotalCost();
}

P.S.: Es gibt auch noch ein paar andere Kleinigkeiten:

• Member initialisiert man in einer Initialisierungsliste
• const bei den Argumenten einer Funktion ist sehr ungewöhnlich, da es Implementierungsdetails der Funktion nach außen trägt. Das mach man normalerweise nur, wenn sich das const auf das Ziel einer Referenz oder eines Pointers bezieht, weil es dann für den Aufrufer relevant ist. Sprich:

void foo(const double bar);

Die Funktion foo kann ihren Parameter intern nicht ändern. Interessiert niemanden

void foo(const double& bar);

Die Funktion foo kann über die Referenz bar den übergebenen Wert nicht ändern. Das ist wichtig.

• Apropos Referenz: Üblicherweise übergibt man komplexe Datentypen bei denen Kopien teuer werden könnten per (const) Referenz. Zum Beispiel die Strings bei dir.

Ansonsten sieht das schon recht gut aus. Die Memberfunktionen machen für meinen Geschmack noch zu wenig, aber das ist ein guter Anfang.

[Gast]

SeppJ schrieb:

Hacker schrieb:

Cart ist ein Zeiger. Nim den Pfeiloperator.

...

Und vergiss nicht, den Speicher freizugeben! Keine Garbage-Collection!! :xmas1:

Nein. Mach das stattdessen gar nicht erst so. Du machst Java in C++. Wenn du new in C++ benutzt ohne einen besonderen Grund, dann machst du etwas falsch.

Wollt ich auch schreiben, erschien mir aber komisch.
Ist es nicht so, das man in Java alle Objekte mit new reservieren muss? (sry, ich hab keine Ahnung von Java)

[Jolka]

Aaaah ok. Danke sehr. Es klappt.
Zwar meckert der Compiler unter CodeBlocks zwar immer noch wegen undefined references herum, aber wenn ich es mit der Konsole kompiliere und linke klappt alles wunderbar. Versteh ich zwar nicht, aber ok.

@Hacker: Ja, bei Java werden alle Objekte immer mit new erzeugt. Und sowas wie reservieren hört man bei Java meines Wissens nach auch garnicht. Java ist nicht so hardwarenah, dass man irgendwas von irgendwelchen Reservierungen zu wissen braucht.

@SeppJ: Dass das const doch nur bei Referenzen Sinn macht leuchtet mir jetzt ein. Danke.

Noch ein paar Fragen:

Der normale Konstruktor sieht also so aus:
CartItem meinItem("Argument String", argumentInt, argumentDouble);

Was macht...
CartItem meinItem = new CartItem("Argument String", argumentInt, argumentDouble);
...dann?

@Hacker: Die Zeiger sind jetzt zwar weg und das Freigeben des Speichers wird damit unnötig, aber nur nochmal eine Frage zum Verständnis: Ich muss bzw. sollte den Speicher von Zeigern auch freigeben, wenn das Programm kurz darauf endet? Wird der Speicher nach Programmende nicht automtisch freigegeben, weil der Prozess zu Ende ist?

Zum Abschluss nochmal mein neue Code. Wenn ihr noch irgendwelche Kleinigkeiten findet, die man besser machen könnte immer raus damit. Außer jetzt die einzelnen CartItem-Objekte nicht in Variablen zwischenzuspeichern, sondern direkt bei der Parameterübergabe zu erzeugen. Das weiß ich ja schon.

CartItem.h

#ifndef CARTITEM_H
#define CARTITEM_H
#include <string>
class CartItem {
    private:
        std::string name;
        int quantity;
        double pricePerUnit;

    public:
        CartItem(const std::string &name, int quant, double price);
        double getCost();
        std::string getName();
        int getQuantity();
        double getPricePerUnit();
        void setQuantity(int n);
        void setPricePerUnit(double price);
        void setName(std::string name);
};
#endif

CartItem.cpp

#include "CartItem.h"

CartItem::CartItem(const std::string &name, int quant, double price) {
    setName(name);
    setQuantity(quant);
    setPricePerUnit(price);
}

double CartItem::getCost() {
    return quantity*pricePerUnit;
}

std::string CartItem::getName() {
    return name;
}

int CartItem::getQuantity() {
    return quantity;
}

double CartItem::getPricePerUnit() {
    return pricePerUnit;
}

void CartItem::setQuantity(int n) {
    quantity = n;
}

void CartItem::setPricePerUnit(double price) {
    pricePerUnit = price;
}

void CartItem::setName(std::string name) {
    this->name = name;
}

ShoppingCart.h

#ifndef SHOPPINGCART_H
#define SHOPPINGCART_H
#include <vector>
#include "CartItem.h"
class ShoppingCart {
    private:
        std::vector<CartItem> contentList;
    public:
        ShoppingCart();
        void add(const CartItem &item);
        double getTotalCost();
        int getNumberOfItems();
        CartItem getItem(int i);
};
#endif

ShoppingCart.cpp

#include "ShoppingCart.h"

void ShoppingCart::add(const CartItem &item) {
    contentList.push_back(item);
}

double ShoppingCart::getTotalCost() {
    double totalCost=0;
    for(unsigned int i=0; i<contentList.size(); i++) {
        totalCost += contentList.at(i).getCost();
    }
    return totalCost;
}

int ShoppingCart::getNumberOfItems() {
    return contentList.size();
}

CartItem ShoppingCart::getItem(int i) {
    return contentList.at(i);
}

main.cpp

#include <iostream>
#include "ShoppingCart.h"
#include "CartItem.h"

using namespace std;

int main() {
    ShoppingCart cart;
    CartItem hundefutter("Hundefutter", 6, 3.20);
    CartItem kekse("Kekse", 4, 1.59);
    CartItem milch("Milch", 1, 0.69);
    CartItem marmelade("Erdbeerkonfituere", 3, 2.19);
    cart.add(hundefutter);
    cart.add(kekse);
    cart.add(milch);
    cart.add(marmelade);
    cout << cart.getTotalCost();
}

[Gast]

1. Speicher gibt man bei Objekten frei, dessen Speicher man vom Heap reserviert hat. Einfachste Form:

int* o = new int[5];

Danach muss, wenn du weißt dass das Programm sich schließt, der Speicher (in diesem Fall mit delete[], wegen Array - google delete operator) freigegeben werden:

delete [] o;

Z.B. hast du ne Klasse, die so aussieht:

#include <stdexcept>

template<class type>
class Array
{
    type* arr;
    std::size_t len;

public:

    Array(const std::size_t& l) : arr(new type[l]), len(l){} //Reservierung

    Array(const Array& ref)
    {
        *this = ref;
    }

    ~Array() { delete [] arr;} //Freigabe

    Array& operator=(const Array& ref)
    {
        this->~Array();
        len = ref.len;
        arr = new type[len];// Reservierung
        for(std::size_t ct(0);ct < len; ++ct)
            arr[ct] = ref.arr[ct];
    }

    type& operator[](const std::size_t& index) throw(std::out_of_range)
    {
        if(index > len - 1)
            throw std::out_of_range("No such index");
        return arr[index];
    }
};

Änder ich jetzt nicht mehr, sollte nur ein Beispiel sein :xmas1:

2. Du musst nur dann Speicher freigeben, wenn du ihn selbst reserviert hast. Bei normalen Variablen, deren Speicher vom Stack kommt haben automatisch das hier:

int a = 0;  // dasselbe wie
auto int a = 0;// auto unnötig

Speicher reservierung und freigabe sind automatisiert (in C++11 übernimmt dieses Schlüsselwort automatic type deduction).

[SeppJ]

Jolka schrieb:

Der normale Konstruktor sieht also so aus:
CartItem meinItem("Argument String", argumentInt, argumentDouble);

Ja, das ist dann ein automatisches Objekt. Das lebt solange wie die eckigen Klammern drumherum und wird, wenn diese verlassen werden, automatisch zerstört. Das ist ungeheuer praktisch, weil das sehr schnell ist und auch meistens das ist, was man möchte. Daher braucht's in C++ auch keinen Garbage Collector. Die Objekte werden streng deterministisch zerstört. Und noch ein ganz dicker Vorteil dieses automatischen Zerstörens: Wenn ich von Zerstören rede, dann meine ich, dass der Destruktor aufgerufen wird. Das heißt, es wird eine Funktion automatisch aufgerufen, wenn das Objekt den Fokus verlässt und diese Funktion kann man selber schreiben. Dies ermöglicht es, Klassen zu schreiben, die automatisch hinter sich aufräumen. Die Standardbibliothek macht das schon so. Dateistreams schließen sich automatisch, Containerklassen geben ihren Speicher frei, usw. Dieses Konzept nennt man RAII.

Was macht...
CartItem meinItem = new CartItem("Argument String", argumentInt, argumentDouble);
...dann?

Hier hingegen übernimmt der Programmierer die Verantwortung für die Lebensdauer des Objekts. Dies kann nützlich sein, wenn man sehr spezielle Anforderungen an die Lebensdauer eines Objekts hat oder auch unter manchen anderen Umständen. Das Objekt wird erst dann und nur dann zerstört, wenn ein passendes delete aufgerufen wird. Außerdem ist die Technik meistens deutlich langsamer, das Standard-new ist ziemlich lahm (es gibt gute technische Gründe dafür, das ist keine böse Absicht). Wenn man so etwas macht, dann kombiniert man dies oftmals mit dem oben genannten RAII-Idiom, indem man nicht einfach einen Pointer auf das Objekt zeigen lässt, sondern eine komplexe Pointerklasse, die das Objekt unter definierten Umständen automatisch zerstört, z.B. wenn der letzte Zeiger auf das Objekt aus dem Fokus geht. Die Standardbibliothek bietet bereits eine Reihe fertiger Pointerklassen für verschiedene Anwendungsgebiete.

@Hacker: Die Zeiger sind jetzt zwar weg und das Freigeben des Speichers wird damit unnötig, aber nur nochmal eine Frage zum Verständnis: Ich muss bzw. sollte den Speicher von Zeigern auch freigeben, wenn das Programm kurz darauf endet? Wird der Speicher nach Programmende nicht automtisch freigegeben, weil der Prozess zu Ende ist?

Nur die automatischen Variablen werden am Ende automatisch freigegeben (außer bei ganz brutalen Programmabbrüchen, z.B. Abstürzen oder dem Aufruf von abort). Wenn es zu einem new kein delete gab, dann wird dieses ausdrücklich nicht freigegeben. Zumindest nicht von deinem Programm. Du darfst aber davon ausgehen, dass ein eventuelles Betriebssystem sich allen Speicher von einem Prozess automatisch zurückholt, wenn dieser beendet. Das muss aber nicht so sein. Bei sehr alten oder bei extrem einfach gebauten Systemen ist das eventuell nicht so. Daher: Mach es immer sauber. Anders ist unsauber und mangelt an Eleganz. Du wirst feststellen, dass es mit den genannten Techniken sehr einfach ist, absolut fehlerfreie Programme zu schreiben. Außerdem wirst du dadurch ein besserer Programmierer.

Und lass dich nicht von Hackers Beispielen verwirren. Ganz besonders new[] (also new für Arrays) brauchst du wirklich überhaupt nie in C++.

[hustbaer]

Hacker schrieb:

Array(const Array& ref)
    {
        *this = ref;
    }

Vor dem *this = ref; erstmal arr = 0 setzen wäre angesagt. Sonst schnalzt es an der Stelle.

Hacker schrieb:

Array& operator=(const Array& ref)
    {
        this->~Array();
        len = ref.len;
        arr = new type[len];// Reservierung
        for(std::size_t ct(0);ct < len; ++ct)
            arr[ct] = ref.arr[ct];
    }

this->~Array(); ... AUA!
Ich bin mir fast sicher dass das laut Standard nicht OK ist.
Und selbst wenn, eine kleine Hilfsfunktion die du hier sowie im Dtor aufrufst wäre wirklich kein Overkill.
Und natürlich ist das ganze nicht Exception-safe. Copy & Swap wäre hier IMO angesagt, bzw. ansonsten etwas in der Art wie...

void Clear()
    {
        delete [] arr;
        arr = 0;
        len = 0;
    }

    Array& operator=(const Array& other)
    {
        Clear();
        arr = new type[other.len]; // erst arr setzen, damit verbleibt das Objekt immer in einem gültigen Zustand, auch wenn es hier schnaltz
        len = other.len;           // würde man erst len setzen, könnte man arr = 0, len = 42 haben - was dann zu Problemen im operator[] führen könnte
        for (std::size_t i = 0; i < len; i++)
            arr[i] = other.arr[i];
    }

Hacker schrieb:

type& operator[](const std::size_t& index) throw(std::out_of_range)
    {
        if(index > len - 1)
            throw std::out_of_range("No such index");
        return arr[index];
    }
};

Und was wenn len == 0? Wieso nicht einfach if (index >= len)?

[314159265358979]

hustbaer schrieb:

void Clear()
    {
        delete [] arr;
        arr = 0;
        len = 0;
    }

    Array& operator=(const Array& other)
    {
        Clear();
        arr = new type[other.len]; // erst arr setzen, damit verbleibt das Objekt immer in einem gültigen Zustand, auch wenn es hier schnaltz
        len = other.len;           // würde man erst len setzen, könnte man arr = 0, len = 42 haben - was dann zu Problemen im operator[] führen könnte
        for (std::size_t i = 0; i < len; i++)
            arr[i] = other.arr[i];
    }

Ist auch nicht Exception-safe. Besser:

void Clear()
{
    delete[] arr;
    arr = 0;
    len = 0;
}

Array& operator = (Array const& other)
{
    type* ptr = new type[other.len]; // Gleich mal als erstes den Speicher allozieren, um die starke Garantie geben zu können
    Clear();
    arr = ptr;
    len = other.len;
    std::copy(other.begin(), other.end(), arr);
    return *this;
}

Der Nachteil besteht wohl darin, dass kurzzeitig der doppelte Speicher gebraucht wird, was dann ziemlich knapp wird, wenn man sehr große solche Arrays verwendet.

hustbaer schrieb:

Hacker schrieb:

type& operator[](const std::size_t& index) throw(std::out_of_range)
    {
        if(index > len - 1)
            throw std::out_of_range("No such index");
        return arr[index];
    }
};

Und was wenn len == 0? Wieso nicht einfach if (index >= len)?

Die Frage sollte eher lauten, was das throw(std::out_of_range) da soll.

[Gast]

Was werfen denn String und Vector, wenn du

std::vector<int> a(10);

a.at(20);

machst?

MEIN Gott, es war nur ein Beispiel. :xmas1:

@hustbaer: an index >= len hab ich doch so schnell nicht gedacht. Es war nur hingekrizelt. Außerdem hast du aber recht, std::size_t ist unsigned...

[unskilled]

Hacker schrieb:

Was werfen denn String und Vector, wenn du

std::vector<int> a(10);

a.at(20);

machst?

Er meinte mit Sicherheit das
type& operator[](const std::size_t& index) throw(std::out_of_range)
es gibt schon 12387 diskussionen über exception specifications, aber bisher kam man glaube ich jedes mal auf den schluss, lieber
type& operator[](const std::size_t& index) /*throw(std::out_of_range)*/
zu schreiben.

bb

edit: copy&swap wäre zu einfach?

#include <stdexcept> 

template<class type> 
class Array 
{ 
    type* arr; 
    std::size_t len; 

public: 

    Array(const std::size_t& l)
    :   arr(new type[l]),
        len(l)
    {}

    Array(const Array& ref)
    :   arr(new type[ref.l]),
        len(ref.l)
    { 
        try
        {
            std::copy(ref.l, ref.l+ref.len, arr);
        }
        catch(...)
        {
            clear();
            throw;
        }
    } 

    void clear()
    {
        delete[] arr;
    }

    ~Array()
    {
        clear();
    }

    void swap(Array& ref)
    {
        using std::swap;
        swap(l, ref.l);
        swap(arr, ref.arr);
    }

    Array& operator=(const Array& ref)
    {
        Array tmp(ref);
        tmp.swap(*this);
        return *this;
    }

    type& operator[](const std::size_t& index)
    {
        assert(index >= len);
        return arr[index]; 
    } 

    type& at(const std::size_t& index) /*throw(std::out_of_range)*/
    {
        if(index >= len)
            throw std::out_of_range("No such index"); 
        return this->operator[](index); 
    } 
};

wer auf die idee kommt, immer l für länge/length zu nutzen... so was unleserliches...
ref find ich auch nicht gerade aussagekräftig, aber ich habs bei swap mal auch so gemcaht - normalerweise nehm ich bei so etwas other .

bb

[Gast]

unskilled schrieb:

Hacker schrieb:

Was werfen denn String und Vector, wenn du

std::vector<int> a(10);

a.at(20);

machst?

Er meinte mit Sicherheit das
type& operator[](const std::size_t& index) throw(std::out_of_range)
es gibt schon 12387 diskussionen über exception specifications, aber bisher kam man glaube ich jedes mal auf den schluss, lieber
type& operator[](const std::size_t& index) /*throw(std::out_of_range)*/
zu schreiben.

bb

Ah, gut! Danke werds mir merken :xmas1:

[SeppJ]

Zumal es nun sogar offiziell deprecated ist, das so zu schreiben.

[hustbaer]

Quatsch, mal wieder, leider.

314159265358979 schrieb:

hustbaer schrieb:

void Clear()
    {
        delete [] arr;
        arr = 0;
        len = 0;
    }

    Array& operator=(const Array& other)
    {
        Clear();
        arr = new type[other.len]; // erst arr setzen, damit verbleibt das Objekt immer in einem gültigen Zustand, auch wenn es hier schnaltz
        len = other.len;           // würde man erst len setzen, könnte man arr = 0, len = 42 haben - was dann zu Problemen im operator[] führen könnte
        for (std::size_t i = 0; i < len; i++)
            arr[i] = other.arr[i];
    }

Ist auch nicht Exception-safe. Besser:

void Clear()
{
    delete[] arr;
    arr = 0;
    len = 0;
}

Array& operator = (Array const& other)
{
    type* ptr = new type[other.len]; // Gleich mal als erstes den Speicher allozieren, um die starke Garantie geben zu können
    Clear();
    arr = ptr;
    len = other.len;
    std::copy(other.begin(), other.end(), arr);
    return *this;
}

Erklär mir mal bitte wo mein Code nicht exception-safe ist.

Er erfüllt die "basic guarantee": das Objekt verbleibt auf jeden Fall in einem gültigen Zustand, und es gibt keine Leaks. Dass der gültige Zustand im Fall eine Exception nicht unbedingt dem entspricht, was sich der Aufrufer erwartet, ist für die "basic guarantee" vollkommen irrelevant.

Dein Code dagegen versucht die "strong guarantee" zu geben, scheitert dabei aber kläglich. Nämlich genau dann, wenn std::copy() eine Exception wirft. Was ja ohne weiteres sein kann, da es sich um ein Template handelt, und type::operator = ja Exceptions werfen kann. Dann ist nämlich ein Teil des Arrays kopiert, ein Teil aber nicht (=Elemente wurden Default-Initialisiert, aber danach nicht zugewiesen).

Wenn man die "strong guarantee" geben will, dann nimmt man am besten Copy & Swap dafür. Was ich ja auch schon angesprochen hatte.

ps: streng genommen hab ich sogar einen Fehler in meinem Code, den du auch nicht ausgebessert hast: es dürfen ja auch Destruktoren Exceptions werfen. Das ist zwar ganz viel pfui, aber grundsätzlich erlaubt. D.h. Clear() könnte (sollte?) man vielleicht folgendermassen anpassen:

void Clear()
    {
        type* to_delete = arr;
        arr = 0;
        len = 0;
        delete [] to_delete;
    }

[hustbaer]

@unskilled:
Naja, copy & swap hatte ich ja schon erwähnt.
Wobei dein Code auch einen Bug hat (neben den Tippfehlern und dem nicht optimalen re-throw im Ctor):

clear() muss schon arr und len auf Null setzen, sonst zerstört es ja die Invarianten.

Und zwecks besserer Optimierbarkeit sollte man den operator = so schreiben:

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

ps: um das re-throw im Ctor loszuwerden muss man wohl ober übel eine Hilfsklasse verwenden, die das im Destruktor erledigt. Diese Hilfsklasse kann man dann entweder als Basisklasse, Member oder lokalen Guard einbinden (ich würde es vermutlich als Member machen).

Der Sinn ist, dass man kein try-catch braucht. Das hat nämlich Vorteile, wenn die Exception nirgends gefangen wird. Dann bleibt der Debugger nämlich dort stehen, wo die "primäre" Exception geworfen wird, und nicht beim "throw;" in deinem catch-Handler.
Weiters führt es (je nach Compiler) dazu, dass beim "freien" Ausführen des Programms kein (teilweises) Stack-Unwinding mehr gemacht wird, bevor das Programm abgebrochen wird, was auch Vorteile haben kann.

[314159265358979]

hustbaer schrieb:

Quatsch, mal wieder, leider.

Es heißt "Quark, mal wieder, leider.". Installier dir das Script oder spar's dir.

hustbaer schrieb:

Er erfüllt die "basic guarantee": das Objekt verbleibt auf jeden Fall in einem gültigen Zustand, und es gibt keine Leaks. Dass der gültige Zustand im Fall eine Exception nicht unbedingt dem entspricht, was sich der Aufrufer erwartet, ist für die "basic guarantee" vollkommen irrelevant.

Warum die basic guarantee geben, wenn man strong geben kann? Depp.

hustbaer schrieb:

Dein Code dagegen versucht die "strong guarantee" zu geben, scheitert dabei aber kläglich. Nämlich genau dann, wenn std::copy() eine Exception wirft. Was ja ohne weiteres sein kann, da es sich um ein Template handelt, und type::operator = ja Exceptions werfen kann. Dann ist nämlich ein Teil des Arrays kopiert, ein Teil aber nicht (=Elemente wurden Default-Initialisiert, aber danach nicht zugewiesen).

Daran habe ich nicht gedacht, da hast du Recht. Trotzdem ist mein Code schon ne Stufe besser als deiner.

hustbaer schrieb:

ps: streng genommen hab ich sogar einen Fehler in meinem Code, den du auch nicht ausgebessert hast: es dürfen ja auch Destruktoren Exceptions werfen. Das ist zwar ganz viel pfui, aber grundsätzlich erlaubt. D.h. Clear() könnte (sollte?) man vielleicht folgendermassen anpassen:

Quatsch. Wenn jemand Exceptions in Dtoren wirft, ist er selbst schuld. Auch die Standardcontainer geben in so einem Fall keine Garantien mehr.

[Michael E.]

hustbaer schrieb:

void Clear()
    {
        delete [] arr;
        arr = 0;
        len = 0;
    }

    Array& operator=(const Array& other)
    {
        Clear();
        arr = new type[other.len]; // erst arr setzen, damit verbleibt das Objekt immer in einem gültigen Zustand, auch wenn es hier schnaltz
        len = other.len;           // würde man erst len setzen, könnte man arr = 0, len = 42 haben - was dann zu Problemen im operator[] führen könnte
        for (std::size_t i = 0; i < len; i++)
            arr[i] = other.arr[i];
    }

Plus Check auf Selbstzuweisung, weil sonst die Selbstzuweisung arr leert.

[Michael E.]

Quark, mal wieder, leider.

314159265358979 schrieb:

Warum die basic guarantee geben, wenn man strong geben kann? Depp.

hustbaer schrieb:

Dein Code dagegen versucht die "strong guarantee" zu geben, scheitert dabei aber kläglich.[...]

Daran habe ich nicht gedacht, da hast du Recht.

Fällt dir nichts aus? Oder warst du zu sehr damit beschäftigt, Schimpfwörter zu wählen? Man sieht mal wieder, dass deine guten Vorsätze nicht von weit her sind.

[314159265358979]

*gähn

[Michael E.]

Man kann kaum glauben, dass du mal rumgeheult hast, dass du nicht verstehst, warum du unbeliebt bist, und dass du ja eigentlich ein ganz Freundlicher bist und in Zukunft nett sein willst.

[hustbaer]

@Michael E.:
Ja, check auf Selbstzuweisung wäre gut. Hab ich nicht dran gedacht. Danke für den Hinweis.