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

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

[314159265358979]

Wie man in den Wald hineinruft, so schallt es wieder raus.

[talktoyou]

314159265358979 schrieb:

Wie man in den Wald hineinruft, so schallt es wieder raus.

Ich hab nichts gegen Leute die Selbstgespräche führen.

[Gast]

talktoyou schrieb:

314159265358979 schrieb:

Wie man in den Wald hineinruft, so schallt es wieder raus.

Ich hab nichts gegen Leute die Selbstgespräche führen.

Ich hab auch nichts gegen Leute die Leute die Selbstgespräche führen auf eine subtile Weise kritisieren, nein ganz und gar nicht :xmas1: :xmas2:

[jklkl]

Ist die Kiddierunde jetzt bald komplett?

[Gast]

jklkl schrieb:

Ist die Kiddierunde jetzt bald komplett?

Jetzt schon. :xmas1:

Edit: Nein, du hast ja recht :seufz:

[unskilled]

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

was sind invarianten? ich konnt auch bei google via invariants nichts auf die schnelle finden. des weiteren sollte es ja reichen, length auf 0 zu setzen
auf anhieb ist mir jetzt auch kein szenario eingefallen, wo es irgendetwas kaputt macht.

tippfehler

assert(index >= len); ist natürlich falsch, sry war aber eher nen flüchtigkeitsfehler als nur ein tippfehler: assert(index < len);

ps: um das re-throw im Ctor loszuwerden muss man wohl ober über eine Hilfsklasse verwenden, die das im Destruktor erledigt. Diese Hilfsklasse kann man dann entweder als Basisklasse, Member oder lokalen Guard einbinden.

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.

hab ich auch noch nie was von gehört; klingt aber höchst interessant. hast du nen link dazu? wäre toll

danke schon mal,
bb