0

Сan you help me fixing this code, The function printIt from exampleA is not used. I used shared pointer and reference_wrapper but il does not look to work fine.

Wrong result:

1;2;23;23;23; 

http://cpp.sh/9yqpq

#include <iostream>
#include <vector>
#include <algorithm>
#include <list>
#include <vector>
#include <iostream>
#include <numeric>
#include <random>
#include <functional>
#include <memory>

struct obj{
    int value;
};


class example
{
    public:
  obj _obj;
  
  obj getVal() {return _obj;}
  
   virtual void printIt(){
    
          std::cout << _obj.value<< ";";

}
};

struct exampleA : public example
{
  public:
    
    virtual void printIt() override{
    
          std::cout <<"+++ "<< _obj.value<< "-";

}
};

class Container {
    
        std::vector<std::shared_ptr<example> > input;

public:

void add (const example& a){
    input.emplace_back(std::make_shared<example>(a));
}


std::vector<std::shared_ptr<example> > getInput()
{
    return input;
}
    
};



int main(int argc, const char* argv[])
{

    example one;
    one._obj = obj{1};
    example two;
    two._obj= obj{2};
    exampleA three;
    three._obj =obj{23};
    
    Container cont;

    
        cont.add(one);
         cont.add(two);
          cont.add(three);
          cont.add(three);
          cont.add(three);



    for (std::shared_ptr<example> entry : cont.getInput() )
    {
      example* ref = entry.get();
      ref->printIt();
    }

    return 0;
}
Jarod42
  • 203,559
  • 14
  • 181
  • 302
Mira
  • 3
  • 1
  • 1
    You probably need a (`virtual`) clone method, you create explicitly `example` actually. – Jarod42 Jul 05 '21 at 11:00
  • In your for cycle, the `entry* ref` is not necessary. Simply use `entry->printIt()` – zerocukor287 Jul 05 '21 at 11:01
  • `input.emplace_back(std::make_shared(a));` yuo are constructing only `example` in the `input` so why are you surprised? In this line implicit copy constructor is used and you have here [object slicing](https://stackoverflow.com/q/274626/1387438). – Marek R Jul 05 '21 at 11:51
  • How to fix it? please! – Mira Jul 05 '21 at 11:53
  • Note that begging in not welcome! You have explanation of problem in comets please try to understand them and fix if by yourself. – Marek R Jul 05 '21 at 12:00

2 Answers2

2

This line:

    input.emplace_back(std::make_shared<example>(a));

always creates new object of type example, argument a is const reference to example so implicit copy constructor of example kicks in and yo have object slicing.

To properly fix it you should pass shared_ptr directly to add function:

    void add(std::shared_ptr<example> a)
    {
        input.emplace_back(std::move(a));
    }

and correct code which uses this function.

Demo

#include <iostream>
#include <vector>
#include <algorithm>
#include <list>
#include <vector>
#include <iostream>
#include <numeric>
#include <random>
#include <functional>
#include <memory>

struct obj {
    int value;
};

class example {
public:
    obj _obj;

    obj getVal() { return _obj; }

    virtual void printIt()
    {

        std::cout << _obj.value << ";";
    }
};

struct exampleA : public example {
public:
    virtual void printIt() override
    {

        std::cout << "+++ " << _obj.value << "-";
    }
};

class Container {

    std::vector<std::shared_ptr<example> > input;

public:
    void add(std::shared_ptr<example> a)
    {
        input.emplace_back(std::move(a));
    }

    std::vector<std::shared_ptr<example> > getInput()
    {
        return input;
    }
};

int main(int argc, const char* argv[])
{

    auto one = std::make_shared<example>();
    one->_obj = obj{ 1 };
    auto two = std::make_shared<example>();
    two->_obj = obj{ 2 };
    auto three = std::make_shared<exampleA>();;
    three->_obj = obj{ 23 };

    Container cont;

    cont.add(one);
    cont.add(two);
    cont.add(three);
    cont.add(three);
    cont.add(three);

    for (std::shared_ptr<example> entry : cont.getInput()) {
        example* ref = entry.get();
        ref->printIt();
    }

    return 0;
}
Nimantha
  • 6,405
  • 6
  • 28
  • 69
Marek R
  • 32,568
  • 6
  • 55
  • 140
1

As noted in the comments, you create example objects that are copies of const example& a. For object3, that a is just a reference to the base part. That's called "slicing".

A simple fix is to copy the actual argument type, using a template:

template <typename EX>
void add (EX const& a){
  input.push_back(std::make_shared<EX>(a));
}

Now, when you call cont.add(three);, template argument deduction will deduce EX==exampleA, and std::make_shared<ExampleA> will create a std::shared_ptr<ExampleA>.

This pointer will then convert to std::shared_ptr<Example> to fit in the vector. But that is still a pointer to an ExampleA object! This does not slice.

MSalters
  • 173,980
  • 10
  • 155
  • 350
  • template here is IMO an overkill. – Marek R Jul 05 '21 at 12:28
  • @MarekR: This allows the calling code to work as-is. Your solution also works, but requires more complexity from the caller. That might not even be possible, for instance when the caller doesn't have a `shared_ptr` to the object being added. – MSalters Jul 05 '21 at 12:32
  • @MarekR: Not an *"overkill"* IMO, just a quick fix. It still have slicing if dynamic type doesn't match static type. – Jarod42 Jul 05 '21 at 12:32
  • @Jarod42: Note that you can use an explicit `add` if needed. Template Argument Deduction is just the default, and often correct. – MSalters Jul 05 '21 at 12:34
  • Yes it is quick fix, but this quick fix is also the overkill since: 1. QP is clearly a beginner and it is to early for him to write own templates. 2. OP learns what is polymorphism and this solution is a distraction from truly understanding its purpose. 3. Such quick fixes are just hiding an actual problem. – Marek R Jul 05 '21 at 12:39
  • @MarekR: You may find that template (compile-time polymorphism) is not for beginners, while run-time polymorphism (OO) is. I disagree. That's why Stack Overflow allows multiple answers. – MSalters Jul 05 '21 at 12:42