-1

I have a class complex and class signal defined. I have overloaded + and - for complex class. signal class is defined with a member of complex type complex *sig_Data; and i have used array subscription for signal as follows

complex &operator[](int i)
    {
        if(i >= range_start && i <= range_end) return sig_Data[zero_pt+i];
        else return complex(0);
    }

zero_pt is used as a reference.

for the operator overloading of + for signal class I have used this

signal operator+(signal &a, signal &b)
{
    int r_start = min(a.range_start, b.range_start);
    int r_end = max(a.range_end, b.range_end);
    int z_pt = max(a.zero_pt, b.zero_pt);
    signal temp(r_start, r_end, z_pt);
    for(int i = r_start; i <= r_end; i++)
    {
        temp[i] = a[i] + b[i];
    }
    return temp;
}

the addition seems to happen correctly here when i checked the values using VC++ in debug but they are not being assigned to temp. I even tried using an assignment overload with the copy-swap idiom (What is the copy-swap idiom).

The constructor used in the signal operator[](int i) function is.

signal(int r_start, int r_end, int z_pt)
    {
        range_start = r_start;
        range_end = r_end;
        zero_pt = z_pt;
        int arr_ind = r_end - r_start;

        sig_Data = new complex [arr_ind];
    }

Please help me identify where I'm going wrong.

A more complete code:

   #include <iostream>
    #include <conio.h>
    #include <string>

    #include <cstdlib>
    #include <cctype>
    #include <cstring>


    using namespace std;


    namespace Complex
    {
        class complex
        {
            double real;
            double imag;

        public:
            complex(double re = 0, double im = 0)
            {
                real = re;
                imag = im;
            }

            complex(complex &t)
            {
                real = t.real;
                imag = t.imag;
            }

            void StrtoComplex(const char *temp)
            {
                int i;

                for(i = 0; i < strlen(temp); i++)
                {
                    if(temp[i] == 'j' || temp[i] == 'i')
                        break;
                }

                real = atof(temp);//takes till the last valid char so after + or whitespace it ignores
                if(*(temp + i - 1) == '-')
                    imag = -atof(temp + i + 1);
                else
                    imag = atof(temp + i + 1);

            }

            friend complex operator+(complex &a, complex &b);
                    friend ostream &operator<<(ostream &s, complex &t);
            friend istream &operator>>(istream &s, complex &t);
            };
        //overloading + to add complex numbers
        complex operator +(complex &a, complex &b)
        {
            complex t;
            t.real = a.real + b.real;
            t.imag = a.imag + b.imag;
            return(t);
        }

        ostream &operator<<(ostream &s, complex &t)
        {
            s<<t.real<<" +j"<<t.imag;
            return s;
        }

        istream &operator>>(istream &s, complex &t)
        {
            std::string temp;

            std::getline(s, temp);
            t.StrtoComplex(temp.c_str());
            return s;
        }
    }

    namespace Discrete
    {
        using Complex::complex;
        class signal
        {
            complex *sig_Data;

            int range_start, range_end, zero_pt;

        public:
            signal()
            {
                sig_Data = NULL;
                range_start = range_end = zero_pt = 0;
            }

            signal(complex i)
            {
                sig_Data = new complex(i);
                range_start = range_end = zero_pt = 0;
            }

            signal(int r_start, int r_end, int z_pt)
            {
                range_start = r_start;
                range_end = r_end;
                zero_pt = z_pt;
                int arr_ind = r_end - r_start;

                sig_Data = new complex [arr_ind];
            }

            void StrtoSig(char *temp)
            {
                int arr_ind = 0;
                char *tok;

                if(!*temp) return;

                tok = temp;
                zero_pt = 0;
                //
                int flag;

                for(int i = 0; i < (flag = strlen(temp)); i++)
                {
                    tok++;
                    if(*tok == '^') zero_pt = arr_ind;
                    if(*tok == ',') arr_ind++;
                }
                range_start = 0 - zero_pt;
                range_end = arr_ind - zero_pt;

                sig_Data = new complex [arr_ind];
                tok = temp+1;
                for(int i = 0; i <= arr_ind; i++)
                {
                    if(*tok == ',') tok++;
                    while(isspace(*tok)) tok++;
                    if(*tok == '^') tok++;
                    sig_Data[i].StrtoComplex(tok);
                    while(*tok != ',' && *tok != '}'&& *tok != '\0') tok++;
                }
            }

            complex &operator[](int i)
            {
                if(i >= range_start && i <= range_end) return sig_Data[zero_pt+i];
                //else return complex(0);
            }


            friend signal operator+(signal &a, signal &b);
                    friend ostream &operator<<(ostream &s, signal &t);
    friend istream &operator>>(istream &s, signal &t);
        };

        //Overloading + operator
        signal operator+(signal &a, signal &b)
        {
            int r_start = min(a.range_start, b.range_start);
            int r_end = max(a.range_end, b.range_end);
            int z_pt = max(a.zero_pt, b.zero_pt);
            signal temp(r_start, r_end, z_pt);
            for(int i = r_start; i <= r_end; i++)
            {
                temp[i] = a[i] + b[i];
            }
            return temp;
        }


            ostream &operator<<(ostream &s, signal &t)
{
    s<<"{";
    for(int i = t.range_start; i <= t.range_end; i++)
    {
        if(i == (t.range_start + t.zero_pt))
            s<<" ^"<<t[i];
        else if(i == t.range_end)
            s<<" "<<t[i];
        else
            s<<" "<<t[i]<<",";
    }
    s<<"}";
    return s;
}

        istream &operator>>(istream &s, signal &t)
        {
            char *ip;
            s>>ip;
            t.StrtoSig(ip);
            return s;
        }
    }

   void main()
{
    using Discrete::signal;
    signal a,b,c;
    a.StrtoSig("{1+i5, ^7+i6}");
    b.StrtoSig("{5+i4, 7+i5}");

    c = a+b;
    cout<<c;
}
Community
  • 1
  • 1
PSK
  • 5
  • 6
  • In your subscription function, you return complex(0) when index is out of bounds. How will you then differentiate between a zero element and incorrect indexing ? – asheeshr Nov 30 '12 at 13:37
  • returning reference to temporaty object `temp` is asking for troubles. `operator+` should return a copy. –  Nov 30 '12 at 13:40
  • @AshRj It does not matter really because I'm assuming the value of the signal out of this bound is zero.... – PSK Nov 30 '12 at 13:41
  • Your `signal::operator[]` is returning a local by reference in the case `i` is outside of the range. Don't do that. And similarly for `complex::operator+`. – Sander De Dycker Nov 30 '12 at 13:42
  • @aleguana I'll consider ur suggestion but the temp is not being assigned even b4 being returned – PSK Nov 30 '12 at 13:43
  • Ok i changed them but it doesn't solve the issue – PSK Nov 30 '12 at 13:48
  • and if fixing that doesn't solve your issue, please post the implementation of the `signal` constructor used by `complex::operator+`. – Sander De Dycker Nov 30 '12 at 13:48
  • the way you changed the code made it worse ... your `signal::operator[]` has to return by reference in order to allow modifying the actual contents. You just can't return a reference to a local. – Sander De Dycker Nov 30 '12 at 13:51
  • @PSK, well it looks like you didn't even try to compile with these modifications. Now your `operator[]` returns a copy which shouldn't compile because it's not an lvalue and `temp[i] = a[i] + b[i]` is illegal –  Nov 30 '12 at 13:52
  • Also the way you check the range in `operator[]` seems wrong; it should be `if(i >= range_start && i < range_end)`, or if you really want both `range_start` and `range_end` to be valid indices you should use `int arr_ind = r_end - r_start + 1;` in the `signal` constructor. – Anders Johansson Nov 30 '12 at 13:55
  • @Sander De Dycker I don't understand what you mean by reference to a local ..are you referring to the `return complex(0)` part? – PSK Nov 30 '12 at 13:58
  • @aleguna It compliled fine with the above changes i made...PS: I didn't understand a word of the rest you said – PSK Nov 30 '12 at 14:00
  • @PSK : yes, that's what I'm referring to. The subject of returning references to locals has been addressed before on SO, eg. : http://stackoverflow.com/questions/4643713/c-returning-reference-to-local-variable – Sander De Dycker Nov 30 '12 at 14:00
  • @SanderDeDycker Ok say my index is not out of this range defined then shouldn't the above code work fine (as it is compling fine in VC++ im assuming its not a syntax or compiler error) – PSK Nov 30 '12 at 14:03
  • several other issues have been pointed out that need to be fixed too. But in order to track down the root cause of your issue, you'll need to post a more complete (compilable) code sample that has the behavior you observe. – Sander De Dycker Nov 30 '12 at 14:08
  • please include a `main` in the code to show how you use these classes. – Sander De Dycker Nov 30 '12 at 14:20
  • @SanderDeDycker I would love to but my program runs some 600+ lines ...if you still want me to I can ... wasn't too sure if you would want to read through all that code – PSK Nov 30 '12 at 14:25
  • just a simple example of adding two signals will do. It doesn't have to be your complete code - just a small (compilable) subset of it that has the behavior you're trying to fix. Leave out everything that is not relevant to the problem, while keeping just enough to show what's going wrong. – Sander De Dycker Nov 30 '12 at 14:31
  • @PSK, don't get rediculous none is going to read all the code you just posted. just give an example of two `signal` objects that cause the problem –  Nov 30 '12 at 14:40
  • Ok added a simpler version ... – PSK Nov 30 '12 at 14:49
  • Whats is the outcome of `StrtoSig` in this case? That's all we need to know, unless you want us to debug your parsing as well –  Nov 30 '12 at 15:11

2 Answers2

0

If you want to be able to modify the class through the operator[], it must return a reference:

complex &operator[](int i)
{
    if(i >= range_start && i <= range_end) return sig_Data[zero_pt+i];
    else throw std::range_error("invalid index to operator[]");
}

When you return a value, it's being copied into a temporary, so assignment to the result of operator[] gets discarded at the end of the expression.

ecatmur
  • 152,476
  • 27
  • 293
  • 366
  • ...i did this but didn't help. Besides someone just suggested me here not to use reference.. – PSK Nov 30 '12 at 13:54
  • @PSK that was about `operator+`, which should return by value, not by reference. `operator[]` should return by reference. – ecatmur Nov 30 '12 at 13:57
  • @PSK : we didn't say not to return by reference ... we said not to return *locals* by reference. There's a difference ;) – Sander De Dycker Nov 30 '12 at 13:58
0

OK, let me mentally debug your code for you

imagine that you've got two signals

signal s1 (5, 20, 8);
signal s2 (3, 25, 9);

now you are trying to add then

signal s = s1 + s2;

What happens is this:

  1. You construct a temp signal:

    signal temp (3, 25, 9);

  2. Which creates an internal array

    sig_Data = new complex [22];

  3. Your loop look like this

    for (i = 3; i < 25; ++i)

  4. For every i your operator[] does this

    if(i >= range_start && i <= range_end) return sig_Data[9+i];

  5. Now imagine a step where i = 15, it will do this

    return sig_Data[9+15];

Which breaches array boundaries which is undefined behavour.

Now, I'll leave it to you to fix this mess.

PS Besies, you have a memory leak, you never delete[] sig_Data. And even if you did you would have got a heap corruption becuase you do a shallow copy in your copy constructor

  • I guess this is my fault but your understanding of my code is wrong... My `range_start` is never greater then zero and my `zero_pt` is simply the count make the range shift with respect to zero ... I'll add tht code in a bit as well – PSK Nov 30 '12 at 14:22
  • @PSK, give an example of two signals your are trying to add which reproduce the problem –  Nov 30 '12 at 14:28