0

Good evening, I've a question about the copy constructor, in particular how can it manage dynamic resources?

For example I've written some code in order to better explain my doubt : given the following Character, Backpack and Item class


    class Character
    {
    public:
      Character() { std::cout << "Character Constructor " << this << std::endl; }
      Character(const Character & other) : inventory(other.inventory) {
        std::cout << "Character Copy Constructor " << this << std::endl;} 
      ~Character() { std::cout << "Character Destructor " << this << std::endl; }

    private:
      Backpack inventory;
    };


    class Backpack {
    public:

      Backpack() { std::cout << "I'm an backpack " << this << std::endl ; }
      Backpack(const Backpack& other) : items(other.items) { std::cout << "I'm a backpack duplicated " << this << std::endl ; }
      ~Backpack() { std::cout << "Backpack Destroyed " << this << std::endl ; }

     private:
    Item items[3];
    } ;


    class Item
        {
        public:
            Item() { std::cout << "I'm an item " << this << std::endl; }
            Item(const Item &) { std::cout << "I'm an item duplicated " << this << std::endl; }
            ~Item() { std::cout << "Item Destroyed " << this << std::endl; }

        private:
        };
      

This version of the code works fine and dandy, but when I try to go from Item items[3]; to Item* items = new Item[3]; the whole program breaks down and all my tentatives to make it call the item copy constructor have failed and most cause leaks. Any tips? TT.TT

Edit. This is the version of "Backpack.hh" I'm trying to fix

    class Backpack
    {
    public:
      Backpack()
      {
        items = new Item[3];
        std::cout << "I'm an backpack " << this << std::endl;
      }
      Backpack(const Backpack &other) : items(other.items)
      {
        items = new Item[3];
        for (int i = 0; i < 3; i++)
          items[i] = other.items[i];
        std::cout << "I'm a backpack duplicated " << this << std::endl;
      }
      ~Backpack()
      {
        delete[] items;
        std::cout << "Backpack Destroyed " << this << std::endl;
      }

    private:
      Item *items;
    };

Also in order to test my code I did a main.cc file

    #include "Character.hh"

    using namespace std;

    int main(){
        Character Char1;
        cout << endl;
        Character Char2(Char1);
        cout << endl;
        
        return 0;
    }
  • 4
    It will help if you also show the non-working code, as it's hard to fix code we can't see. Also have a read of [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three); it's probably a duplicate when we see the non-working code. – Richard Critten Dec 12 '21 at 17:04
  • @RichardCritten I've just finished reading it, but when I try to apply it to my case it gets a bit messy. – Noob lvl 1 Dec 12 '21 at 17:11
  • `Backpack(const Backpack &other) : items(other.items)` -- Explain in your own words what you are doing on that line, and especially the `items(other.items)` part of that line. – PaulMcKenzie Dec 12 '21 at 17:13
  • 1
    Best avoid naked `new` and `delete`. In this case, you may consider std::vector. If you really need to manage memory, then std::unique_ptr is your friend. This will give you errors where you forget the rules of 0/3/5 – Michael Veksler Dec 12 '21 at 17:13
  • 1
    could you try to show us the compile warnings if any, and the runtime error, and maybe a valgrind output if you are having leaks. – LTBS46 Dec 12 '21 at 17:13
  • 1
    The `items(other.items)` in your pointer version is redundant because it's immediately overwritten by `items = new Item[3];` in the body of your constructor, but it doesn't look like there's anything actually wrong with the copy constructor. It's possible the lack of a copy _assignment operator_ are causing problems. – Nathan Pierson Dec 12 '21 at 17:13
  • @Nooblvl1 You failed to add a user-defined assignment operator. What happens here: `Backpack b1; Backpack b2; b2 = b1;`? – PaulMcKenzie Dec 12 '21 at 17:16
  • @PaulMcKenzie I'm writing a copy constructor and giving him some values in order to initialize. When I define Items as a static array, then that copy constructor (for arrays) is called and the resource is managed "correctly" (ie. it prints "item duplicated!") but when I define Items as a pointer, then I have no idea what items(other.items) does. About the second question, I was thinking of fixing that right after I manage this, but I'd have to apply the rule of three and also define the operator=(). – Noob lvl 1 Dec 12 '21 at 17:18
  • The question now contains multiple different versions of the same code, and lacks a [mre]. This is confusing. Please [edit] your question according to the instructions for creating a [mre]. Capsule summary: anyone in the world should be able to cut/paste the shown code into an empty file, compile, run, and reproduce your problem. – Sam Varshavchik Dec 12 '21 at 17:20
  • 1
    You never see "item duplicated" get printed in your second version because you never call its copy constructor. `items = new Item[3];` default-constructs three `Item` objects. Then in your for loop, you _assign_ `items[i] = other.items[i];`, which uses `operator=` and not `Item::Item(const Item&)`. – Nathan Pierson Dec 12 '21 at 17:20
  • @NathanPierson what would be the correct way to call `Item::Item(const Item&)` ? – Noob lvl 1 Dec 12 '21 at 18:05
  • @NathanPierson -- Write a user-defined assignment operator. It is ridiculously simple, as it would be a one-line `swap` call. – PaulMcKenzie Dec 12 '21 at 23:26
  • @PaulMcKenzie You're the best! Thanks!! :) – Noob lvl 1 Dec 13 '21 at 13:15

0 Answers0