0

I implemented composite design pattern for calculating the nested mathematical expressions modeled in a binary tree. I need to hold two base class pointers *left,*right in my derived composite class. Problem arises when I pass the arguments of composite class constructor as rvalues and get segmentation fault, as if evaluate() member function is unknown. It works well, when arguments are passed as lvalues. Here is my whole code. Appreciate any help.

#include <iostream>
using namespace std;
typedef int (*operation)(int, int);
int add(int a, int b){
    return a + b;
}
int subtract(int a, int b){
    return a - b;
}
    class base_object{
        public:
            virtual int evaluate() = 0;
            virtual ~base_object(){}
    };
    class primitive: public base_object{
        private:
            int value;
        public:
            primitive(int _value): value(_value){}
            primitive(primitive&& primitive):value(std::move(primitive.value)){}
            int evaluate() override{
               return value; 
            }
    };
    
    class composite: public base_object{
        private:
            base_object *left, *right;
            operation op;
        public:
         explicit composite() = delete;
         composite(base_object&& _left, base_object&& _right, operation _op):
                    left(&_left), right(&_right), op(_op){
                    std::cout<<"composite moving constructor was called"<<std::endl;
         }
         composite(base_object *_left, base_object *_right, operation _op):left(_left), right(_right), op(_op){}
         int evaluate() override{
                return op(left->evaluate(), right->evaluate());
         }
    };
int main(){//calculate expression ((10+2)-5)
    composite exp1 = composite(primitive(10), primitive(2), &add);
    composite exp2 = composite(std::move(exp1), primitive(5), &subtract);
    int result = exp2.evaluate();
    std::cout<<"final result of expression="<<result<<std::endl;
    return 0;   
}
8
  • 1
    left is a pointer to the constructor arguments which are destroyed after the constructor ends, use unique_ptr or shared_ptr instead. Commented Jan 19, 2024 at 15:59
  • left and right must be owning pointers. That is, std::unique_ptr (+ std::make_unique()) in modern C++. (Or new & delete, but that's bad practice outside of custom containers/smart pointers, unless you're just trying to learn how new works.) Commented Jan 19, 2024 at 15:59
  • Also: 1. You don't need primitive(primitive&& primitive), the compiler will generate it for you. 2. Don't write explicit default constructors (i.e. ones without parameters), who taught you that? They are mostly useless, and cause some obscure issues. Commented Jan 19, 2024 at 16:00
  • 1
    A possible solution might be: class composite { /*...*/ std::unique_ptr<base_object> left, right; /*...*/ }; with a constructor like composite(std::unique_ptr<base_object> _left, std::unique_ptr<base_object> _right, operation _op) : left(std::move(_left)), right(std::move(_right)) {} and then finally in main() you would have composite exp1{std::make_unique<primitive>(10), std::make_unique<primitive>(2), add}; and the like. Commented Jan 19, 2024 at 21:01
  • 1
    To pass exp1 into exp2, you have to make it dynamic from the start, such as std::unique_ptr<composite> exp1{std::make_unique<composite>(std::make_unique<primitive>(10), std::make_unique<primitive>(2), add)}; composite exp2{std::move(exp1), std::make_unique<primitive>(5), subtract};. Now exp2 owns exp1. To pass exp2 somewhere further, rinse and repeat, allocate it instead of having it on the stack… Commented Jan 19, 2024 at 21:06

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.