Keskustelu: Ohjelmointikysymykset: C++: ongelma dynaamisen muistinvarauksen ja operaattorien ylikuormituksen kanssa

En ole ihan varma otsikosta mutta kokeillaan.
Minulla siis on vektori, joka sisältää osoittimia toiseen luokkaan. Aluksi se on tietenkin tyhjä, ja niitä osoittimia sinne olisi tarkoitus lisäillä operaattorilla +.
koodi näyttää hieman lyhennettynä tältä:

class itemHolder : public obj{//aka laatikko, reppu jne.
    private:
        vector<item*> items;//item on siis esineluokka

        int lkm;

    public:
        //muodostimet tässä, asiaankuulumattomina jätin ne pois

        itemHolder operator + (item const& i) {//tässä tämä ongelmakohta
            items.push_back(new item(i));
            lkm++;
        }
        /* itemit haetaan valmiista esinetaulusta:
         * item esine_x(/*iso joukko alustustietoja*/);
        /*
         *****
         *itemHolder crate;
         *crate + esine_x;//kaataa ohjelman välittömästi
         */

        ~itemHolder(){
            for(int i = 0; i < items.size(); i++){
                delete items[i];//toimiiko vektoriin delete [] items;?? offtopic
            }
        }
};

eli siis peliä varten esineensäilytyslaatikoita ja invi systeemi kasaantumassa..
on ehkä hiukan epäselvästi esitetty mut kai siit selvää saa.
edit::
korjaus kommentointisyntaksiin

itemHolder operator + (item const& i) {//tässä tämä ongelmakohta
            items.push_back(new item(i));
            lkm++;
        }

Pitäisikö tuon olla pikemminkin näin?

itemHolder operator + (item const& i) {//tässä tämä ongelmakohta
            items.push_back(item(i));
            lkm++; //tämä on minusta turha, koska lukumäärä selviää myös items.size();
        }

Menikö ajatus oikein?

Toivottavasti tiedät itse, mitä koodisi on tarkoitus tehdä. Nyt siis annat operaattorille viittauksen olioon ja luot uuden kopion, jonka osoitteen sitten lisäät vektoriin. Muuttuja int lkm vaikuttaisi turhalta, vai onko jokin kohta, jossa se eroaa arvosta items.size()? Tuhoamisen teet aivan oikein (ja esittämäsi toinen tapa ei toimi; vektorissa on taulukollinen osoittimia, kun taas taulukko on vain yksi osoitin, joka osoittaa useampaan peräkkäiseen olioon).

Ainoa näkyvä virhe on operaattorin palautusarvo. Olet määritellyt, että se palauttaa itemHolder-luokan instanssin. Et kuitenkaan palauta tällaista oliota, ja tähän ohjelma ehkä kaatuukin. Jos käsket kääntäjän varoittaa kaikista mahdollisista virheistä, se luultavasti ilmoittaa asiasta näin: "warning: control reaches end of non-void function".

Jos palauttaisit luokan instanssin, se luotaisiin palautusta varten ja tuhottaisiin heti rivin crate + esine_x jälkeen. Jos se olisi kopio tuosta crate-oliosta, kaikki esineet tuhottaisiin samalla. Kuitenkin osoittimet jäisivät yhä crate-olioon, jolloin ohjelma tietenkin kaatuisi, kun näitä yritettäisiin käyttää. Siksi oikea tapa onkin palauttaa viittaus eli tavallaan sama olio uudestaan. Näin toimivat esimerkiksi standardikirjaston virrat (cin, cout, fstream, ...).

luokka & operator + (int x) {
  // ...
  return *this;
}

Muista pitää huolta, että kun itemHolder-objektista poimitaan tavara, sen osoitin siirtyy johonkin järkevään paikkaan. Voisit ehkä harkita myös auto_ptr-luokan käyttöä, se on suunniteltu tilanteisiin, joissa tiettyyn olioon pitää olla vain yksi vakituinen osoitin, joka aina lopuksi tuhotaan.

Mahtavaa, kiitän.
alkoi toimimaan, seuraavanlaisella koodilla:

itemHolder & operator + (item const& i) {
    items.push_back(new item(i));
    return *this;
}

Tosin tuo yksi vakituinen osoitin kuulostaa jotenkin paljon yksinkertaisemmalta ja järkevämmältä.. =/ ilmeisti ajatuksena jotain tällaista:

itemHolder & operator + (item const* i) {
    items.push_back(i);
    return *this;
}
//**********
crate + &esine_x;

no juuh.. kaipa se on parempi saada hyvätkin ideat myöhään kuin ei milloinkaan =/

Tarkemmin vilkaistuani perun puheeni, auto_ptr ei välttämättä toimisikaan aivan halutulla tavalla. Osoittimilla leikkimisen tueksi suosittelen kuitenkin jotain ohjelmallista muistivuotojen seurantaa. Voit esimerkiksi lisätä luokkaan staattisen laskurin, jonka arvoa muutat muodostimessa ja tuhoajassa, jolloin saat pidettyä kirjaa objektien määrästä ja huomaat, jos ne tuntuvat aina vain lisääntyvän. Virheitä nimittäin sattuu todella helposti. :)

okey teenpä niin

Metabolix kirjoitti:

Tarkemmin vilkaistuani perun puheeni, auto_ptr ei välttämättä toimisikaan aivan halutulla tavalla.

Tässähän on se juttu, että auto_ptr:iä ei saa sijoittaa containereihin, esimerkiksi map:iin tai vector:iin. Containereihin sijoitettavien asioiden pitää olla copy constructible ja assignable, mitä auto_ptr ei ole.

Itse en käyttäisi overloadattua +-operaattoria asian lisäämiseksi johonkin containeriin, kun tuo plussa kuitenkin viittaa vähän niin kuin matemaattiseen operaatioon.

Samoin sen lisäysfunktion toteuttaisin vähän eri tavalla, riippuen tietenkin siitä, mitä oikeasti halutaan tehdä. Jos joku muu "omistaa" sen esineen, niin sitten tekisin sen vaikka näin:

itemHolder &hold(item &itm) {
    item_ptrs.push_back(&itm);
    return *this;
}

Jos taas esineet kopioidaan, niin kuin alkuperäisessä koodissa, kirjoittaisin sen näin:

itemHolder &hold(item const &itm) {
    items.push_back(itm);
    return *this;
}

Tjsp. Varmaan monella on vielä joitakin omia nokkelia virityksiään. Mutta paljaitten pointtereiden kanssa en haluaisi ruveta turaamaan, sillä niiden kanssa eritoten

Metabolix kirjoitti:

Virheitä nimittäin sattuu todella helposti.