Destruktoreinsatz korrekt?

Firefighter

Hallo,

ich habe folgendes vorliegen:

namespace BankSimulation
{
    class Transaction
    {
        private:
        Account const* firstAccount;
        Account const* secondAccount;
        float money;
        public:
            Transaction(Account const* firstAccount,Account const* secondAccount,float value);
            ~Transaction();
            void print(std::ostream& os) const;
    };

}

Und die implementation

BankSimulation::Transaction::Transaction(Account const* firstAccount,Account const* secondAccount, float money)
{
    this->firstAccount = firstAccount;
    this->secondAccount = secondAccount;
    this->money = money;
}
BankSimulation::Transaction::~Transaction()
{
    delete this->firstAccount;
    delete this->secondAccount;
}
void BankSimulation::Transaction::print(std::ostream& os) const
{
    os<<this->firstAccount->getAccountId()<<"->"<<this->secondAccount->getAccountId()<<std::endl;
}

Wenn ich in der main nun folgendes mache:

int main()
{
    Account *a = new Account(100,100);
    Account *b = new Account(100,100);

    Transaction t(a,b,100);

    std::cout<<t<<std::endl;

    return 0;
}

Ist es dann legitim die Zeiger auf die Accounts im Transaction Destruktor zu deleten oder sollte ich das doch in der Main machen?

SeppJ

Das darfst du tun, halte ich aber für keine gute Idee, da man deine Transaktion dann nur mit Heap-Objekten benutzen kann. Und gibt es einen guten Grund dafür? Von einer Transaktion erwarte ich nicht, dass sie mir das Konto auflöst!

drakon

Das ist hier eigentlich die Aufgabe der main. Da du da auch die new's hast.

Stell dir dazu z.B vor, dass jemand das hier macht:

int main()
{
    Account a = Account(100,100);
    Account b = Account(100,100);

    Transaction t(&a,&b,100);

    std::cout<<t<<std::endl;

    return 0;
}

Und das ist sogar sehr wahrscheinlich. Wenn du da dann delete auf den Speicher aufrufst, dann hast du ein Problem.

Auch in der Klasse würde ich keine Zeiger benutzen, sondern Referenzen.

class Transaction
    {
        private:
        Account const& firstAccount;
        Account const& secondAccount;
        float money;
        public:
            Transaction(Account const& firstAccount,Account const& secondAccount,float value);
            ~Transaction();
            void print(std::ostream& os) const;
    };
...
BankSimulation::Transaction::Transaction(Account const& firstAccount_,Account const& secondAccount_, float money)
:
firstAccount(firstAccount_),
secondAccount(secondAccount_)
{
    this->money = money;
}
BankSimulation::Transaction::~Transaction()
{
 // destruktor braucht man eigentlich gar nicht mehr
}
void BankSimulation::Transaction::print(std::ostream& os) const
{
    os<<this->firstAccount.getAccountId()<<"->"<<this->secondAccount.getAccountId()<<std::endl;
}

Firefighter

Das ist natürlich nen Gutes Argument. Da hab ich so sehr an das Bereinigen des Speichers gedacht das ich den Semantischen Aspekt komplett vergessen hab.

Dank dir, dann werd ich das ohne Const-Pointer machen und da nur Stack-Objekte reingeben.

Man wie doof muss man sein:) Danke

Nexus

Warum ist Transaction eine Klasse und keine Funktion?

Firefighter

Wir sollen ne Art protokollierung aller Überweisungen in einem Simulationsdurchgang machen und da dacht ich mir ich pack das inne extra Klasse und Speicher das innem std::vector<Transaction> in der Banksimulation später.

Nexus

Ah, okay. Ich dachte nur, dass wenn sowas

Account a(100,100); 
Account b(100,100); 

Transaction t(&a,&b,100);

der Haupteinsatzzweck ist, eine Funktion wahrscheinlich geeigneter wäre. Aber für Zwischenspeicherung ist eine Klasse okay.

Firefighter

Nee das wäre etwas eigenartig Da wäre ne Funktion wirklich besser