0

I am creating a binary tree representation of a mathematical expression for performing differentaiton and integration on it.

I am trying to overload the + operator for my Symbol class. It works fine for:

Symbol("x") + Symbol(2.3);

The issue arises when I want to support expressions like:

Symbol("x") + 2.3;

where a Symbol object is added to a double(floating point).

I have implemented two approaches: Approach 1 (Works but has Code Duplication)

Symbol operator+(double other){
Symbol *temp = new Symbol(other);
if (this->is_constant) {
double value = this->value + temp->value;
Symbol temp2(value);
return temp2;
}

    Symbol temp2;
    temp2.op = '+';
    temp2.left = this;
    temp2.right = temp;
    
    return temp2;

}

This works, but the logic for adding two Symbol objects is almost identical to this function, leading to redundant code.

Approach 2 (Causes Segmentation Fault) To reduce redundancy, I tried:

Symbol operator+(double other){     
    Symbol *temp = new Symbol(other);     
    return *this + *temp;  // Causes segmentation fault when try to access other later
}

Symbol("x") + 2.3

However, this leads to a segmentation fault when try to access this *temp (2.3) object in main fucntion because temp is dynamically allocated but temp pass copy of that object and its scope lost after this function ends.

So, i main motive of posting this question is to know is there any better way to implemnt this?

Here is complete code for reference:

#include <iostream>
#include <memory>
class Symbol{
    public:
    std::string name = "";
    bool is_constant = false;
    bool is_expression = false;
    char op;
    double value = 0.0;
    Symbol* left = nullptr;
    Symbol* right = nullptr;
    

    Symbol() {is_expression = true;}
    Symbol(std::string name) {this->name = name;}
    Symbol(double value) {this->value = value; this->is_constant = true;}
    ~Symbol() {};

    Symbol operator+(Symbol other){
        if(this->is_constant & other.is_constant){
            double value = this->value + other.value;
            Symbol temp(value);
            return temp;
        }

        Symbol temp;
        temp.op = '+';
        temp.left = this;
        temp.right = &other;

        return temp;
    }

    Symbol operator+(double other){
        Symbol *temp = new Symbol(other);
        if(this->is_constant){
            double value = this->value + temp->value;
            Symbol temp2(value);
            return temp2;
        }

        Symbol temp2;
        temp2.op = '+';
        temp2.left = this;
        temp2.right = temp;

        return temp2;
        // // uncomment below line and  comment above function code and this will give segmentation fault
        // return *this + *(new Symbol(other)); 
    }

    void print(){
        if(this->is_expression){
            std::cout << "( ";
            this->left->print();
            std::cout << this->op << " ";
            this->right->print();
            std::cout << ") ";
            return;
        }

        if(this->is_constant){
            std::cout << this->value << " ";
            return;
        }

        std::cout << this->name << " ";

    }
        
};
/*
    z
  /  \    
 x   (2.3)
*/

int main(){
    Symbol x("x");
    Symbol z = x  + 2.3; 

    z.print();    
}
5
  • I don't know if this is the only problem, but the line 28 (temp.right = &other;) looks bad. Yor intention is for temp.right to point to other, but other goes out of scope in the very next line! That is, as soon as the return statement in line 30 is hit, temp.right points to invalid data. That could be a cause of the Segmentation Fault that you're seeing. Commented Feb 26 at 18:28
  • Out of curiosity, why are you even defining the Symbol operator+(double other) method? The other method (Symbol operator+(Symbol other)) should handle a double just fine. If you're wondering why, it's because you already defined a constructor that takes a double. So if you pass a double into Symbol operator+(Symbol other), the Symbol(double value) constructor will convert the double to a Symbol, doing the conversion for you. Commented Feb 26 at 18:46
  • 1
    You have both memory leaks (where you allocate a Symbol on the heap but then end up not using and forgetting the pointer) and dangling pointers (where you save an address of a temporary, then that temporary is destroyed). Before worrying about style, you need to reconsider your design, paying careful attention to ownership - who owns what, when is memory allocated and destroyed, how do you ensure that pointers don't outlive the objects they point to. Commented Feb 27 at 1:20
  • @J-L The goal is to make writing mathematical expressions as natural as possible. It's better to allow expressions like: Symbol('x') + 2.3 Instead of forcing explicit conversions: Symbol('x') + Symbol(2.3) This way, we can write functions naturally, e.g., f(x) = x + 2.3 Commented Mar 14 at 10:52
  • @AdityaVinchhi, I agree that allowing expressions like Symbol('x') + 2.3 is better. But you don't need to define a Symbol operator+(double other) method to support it. If you have trouble believing that, try commenting out the Symbol operator+(double other) method and see if the line Symbol('x') + 2.3 still compiles. Commented Mar 17 at 19:49

1 Answer 1

0

Have you tried this?

class Symbol {
  // ...
  friend Symbol operator+(const Symbol& a, const Symbol& b)
  {
     //...
  }

  // this template will only work if Symbol is constructible from a T 
  template <typename T>
  friend Symbol operator+(const Symbol& a, T&& b)
  {
    return a + Symbol(std::forward<T>(b)); // lets Sympol{} + Symbol{} do the work
  }

};

Sign up to request clarification or add additional context in comments.

Comments

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.