need little help of bug fixers.

here is a code which add data of two classes using operator overloading. But it hangs due to double calling of destructor for same pointer. But i think it should not so.please tell me how to remove this problem from this code.(error is most probably in red region)

Code:

---

#include<iostream>
using namespace std;
class x
{
    private:
    int *******;
    int* pointer;

    public:
    x(int,int);
    x(const x&);
    ~x();
    x operator+(x x1) const;
    void get() const;
    int geti()
    {
        return *******;
    }
    int getp()
    {
        return (*pointer);
    }
    void change(int a,int b)
    {
    *******=a;
    pointer=new int(b);
    cout<<"\t\t"<<pointer<<endl;
    cout<<"change"<<endl;
    }
};
x::x(int *******x=1,int pointerx=1)
{
    try
    {
        *******=*******x;
        pointer=new int(pointerx);
        cout<<"constrctor"<<endl;
    }
    catch(bad_alloc &ex)
    {
        cout<<"ERROR "<<ex.what()<<endl;
    }

}
x::x(const x & x5 )
{
    *******=x5.*******;
    int*tmp=new int(*(x5.pointer));
    pointer=tmp;
    cout<<"copy constructor"<<endl;
}
x::~x()
{
    cout<<"destructor  ";
    cout<<*******<<endl;
    delete pointer;
}
void x::get() const
{
    cout<<*******
    <<endl
    <<*pointer
    <<endl;
}
/*x x::operator+(x xn) const
{
    cout<<"we get this data\n";
    cout<<*******<<endl<<*pointer<<endl;
    xn.get();
    cout<<"that's all\n";
    int tmp*******=xn.*******+*******;
    int tmppointer=((*(xn.pointer))+(*(pointer)));

    x *returnx=new x(tmp*******,tmppointer);
    cout<<"addition coplete"<<endl;
    return (*returnx);
}*/
x operator+(x,x);
int main()
{
    x x1(1,1);
    x x2(10,20);
    x1.get();
    x2.get();
    x x3;
    x3=x1+x2;
    x1.get();
    x2.get();
    x3.get();
    cout<<"sum of 2 and 1 is "<<2+1<<endl;
    return 0;
}
x operator+(x x1f,x x2f)
{
    cout<<"add function..."<<endl;
    int xir=x1f.geti()+x2f.geti();
    int xprv=x1f.getp()+x2f.getp();
    cout<<xir<<"\t"<<xprv<<endl;
    x xr(xir,xprv);
    cout<<"returning..."<<endl;
    return xr;
}
[/color]
//why these guys(codeguru mangers)
put ******in place of      int i ger (remove space )
this is not abusive word)???????all the places where you see
******  in this post i write their inti ge r(without space)

---

as you can see their are two overload for addition but one is hidden. I hide that because in returning of that function destructor is called for all the class x variable and all pointer loose their data.
If i call any class function(operator+ of class x) using class variable then it's destructor(main caller like x1.get() not destructor) should not called at the last of that function but in case of operator+ (of class x) this happen destructor is called for all.why. Is it done in special cases like operator overloading???

There should not any error in second operator+(after main function). Error is due to calling of destructor of xprv is deleted at the end of this function but it should not matter pointer value in xr because this is created dynamically in constructor.(but i am wrong why???)
thanks to all helper.

1) just change the name so the "******" don't show up. It is difficult
to follow the code otherwise.

2) you need to supply operator =.

If you need one of : copy constructor , operator = , destructor ...
you usually need all 3.

Quote:

---

Originally Posted by Philip Nicoletti

1) just change the name so the "******" don't show up. It is difficult
to follow the code otherwise.

2) you need to supply operator =.

If you need one of : copy constructor , operator = , destructor ...
you usually need all 3.

---

philip I have no intension to show up i put real code but "int ig ers"(remove space) word s appearing as ******* I don't know why this happening.
(Type int ig er(without space ) in any post you will see that it appear as ******* it's not my fault.)

I don't understand second point can you please explain in better way(briefly).

Quote:

---

Originally Posted by vkash

I don't understand second point can you please explain in better way(briefly).

---

If you do not know what a copy-assignment operator is, then you need to know the basics. You should not be attempting to write overloaded operators if you don't understand these principles:

Code:

---

class foo
{
    private:
        int *ptr;

    public:
        foo() { ptr = new int; }
        ~foo() { delete ptr; }
};

int main()
{
  foo f1;
  foo f2 = f1;  // initialization using copy construction
  foo f3;
  f3 = f1;  // assignment
}

---

This code has memory leaks and attempts to delete the same pointer three times. What is missing from the foo() class to make the main code run correctly?

1) It is missing a user-defined copy constructor to properly handle the pointer member whein initializing a new object from an existing object.

2) It is missing an operator = (copy assignment operator) to handle assigning an existing object from an existing object.

So let's skip the asterisks for now, as all that other code you posted contains mostly noise that has nothing to do with your issue. Just fix the class above so that it works correctly. Once you do that, then it should be understandable as to what makes a class have correct copy semantics.

Regards,

Paul McKenzie

Quote:

---

Originally Posted by Paul McKenzie

If you do not know what a copy-assignment operator is, then you need to know the basics. You should not be attempting to write overloaded operators if you don't understand these principles:

Code:

---

class foo
{
    private:
        int *ptr;

    public:
        foo() { ptr = new int; }
        ~foo() { delete ptr; }
};

int main()
{
  foo f1;
  foo f2 = f1;  // initialization using copy construction
  foo f3;
  f3 = f1;  // assignment
}

---

This code has memory leaks and attempts to delete the same pointer three times. What is missing from the foo() class to make the main code run correctly?

1) It is missing a user-defined copy constructor to properly handle the pointer member whein initializing a new object from an existing object.

2) It is missing an operator = (copy assignment operator) to handle assigning an existing object from an existing object.

So let's skip the asterisks for now, as all that other code you posted contains mostly noise that has nothing to do with your issue. Just fix the class above so that it works correctly. Once you do that, then it should be understandable as to what makes a class have correct copy semantics.

Regards,

Paul McKenzie

---

I have modified code see this. this time with no *******

Code:

---


#include<iostream>
using namespace std;
class x
{
    private:
    int i;
    int* p;

    public:
    x(int xi,int xp);
    x(const x&);
    ~x();
    void get() const;
    int geti() const ;
    int getp() const;
    x& operator=(const x&);
    x& operator+(const x&);
    void change(int ,int);
};
x::x(int xi=1,int xp=1)
{
    cout<<"constructor start..."<<endl;
    i=xi;
    p=new int(xp);
    cout<<"constructor  end..."<<endl;
}
x::x(const x& xc)
{
    cout<<"copy constructor start ..."<<endl;
    int it=xc.geti();
    int pt=xc.getp();
    this->change(it,pt);
    cout<<"copy constructor  end  ..."<<endl;
}
x::~x()
{
    cout<<"destructor start..."<<endl;
    delete p;
    cout<<"destructor  end ..."<<endl;
}
void x::get() const
{
    cout<<"void get() start ..."<<endl;
    cout<<"*******:  "<<i<<endl;
    cout<<"pointer:  "<<*p<<endl;
    cout<<"void get()  end ... "<<endl;
}
int x::geti() const
{
    return i;
}
int x::getp() const
{
    return (*p);
}
void x::change(int a,int b)
{
    cout<<"void change() start ..."<<endl;
    i=a;
    delete p;
    int*op=new int(b);
    p=op;
    cout<<"void change()  end  ..."<<endl;
}
x& x::operator=(const x& xn)
{
    cout<<"operator= start..."<<endl;
    int it=xn.i;
    int pt=xn.getp();
    change(it,pt);
    cout<<"result"<<endl<<i<<"\t"<<*p<<endl;
    return *this;
}
x& x::operator+(const x& xop)
{
        cout<<"opearator+ start...\n";
    int itmp=this->geti()+xop.geti();
    int ptmp=this->getp()+xop.getp();
    x* xreturn=new x(itmp,ptmp);
        cout<<"operator+ end  ...\n";
    return *xreturn;
}
int main()
{
  //x* x1=new x(10,20);
    x x2;
    /*cout<<"first round(view orignal)"<<endl;
    x1->get();
    x2.get();
    cout<<"\n\nsecond round(x2=x1 then view)"<<endl;
    x2=(*x1);
    x1->get();
    x2.get();
    cout<<"\n\nthird round(delete x1)"<<endl;
    delete x1;
    //x1->get();
    x2.get();
    int i;
    cout<<"read then clear screen(press any key)";
    cin>>i;
    system("cls"); */
    x x3;
    x x4(12,13);
    x3=(x4+x2);
    cout<<"result"<<endl;
    x3.get();
    return 0;
}

---

tell me is it causing memory leak. because destructor for highlighted part of program is not called But destructor for x3 is called. so is there any memory leak or not?
If there is any other error then please tell me about that.

thanks paul and philip for helping.

Regarding memory leaks (and how to detect them) see this thread http://www.codeguru.com/forum/showthread.php?t=507470

Quote:

---

Originally Posted by S_M_A

Regarding memory leaks (and how to detect them) see this thread http://www.codeguru.com/forum/showthread.php?t=507470

---

very good link.
i tr these code lines in my code

Code:

---

//at head
#define _CRTDBG_MAPALLOC
#include <stdlib.h>
#include <crtdbg.h>
. ..  .. . . .  .. . .. .  .
 . .. . . . . . . . .

//just before end of return of main function
_CrtDumpMemoryLeaks();

---

results in output window is amazing it says that a lot of memory leaked

Code:

---

Detected memory leaks!
Dumping objects ->
{158} normal block at 0x00354B58, 4 bytes long.
 Data: <!  > 21 00 00 00
{157} normal block at 0x00354CF8, 4 bytes long.
 Data: <!  > 21 00 00 00
{156} normal block at 0x00354CB0, 8 bytes long.
 Data: <    L5 > 16 00 00 00 F8 4C 35 00
{155} normal block at 0x00354B98, 4 bytes long.
 Data: <    > 0D 00 00 00
{141} normal block at 0x00354A38, 4 bytes long.
 Data: <    > 14 00 00 00
Object dump complete.
The program '[2408] class.exe: Native' has exited with code 0 (0x0).

---

i have done all things as written in web page but it is not giving information about the line where memory leak occur. can you tell me why?

Quote:

---

Originally Posted by vkash

very good link.
i tr these code lines in my code

---

You need to fix other issues with your program.

Code:

---

x::x(const x& xc)
{
    cout<<"copy constructor start ..."<<endl;
    int it=xc.geti();
    int pt=xc.getp();
    this->change(it,pt);

---

Code:

---

void x::change(int a,int b)
{
    cout<<"void change() start ..."<<endl;
    i=a;
    delete p;

---

You are deleting an uninitialized pointer.

As to your other issues:

Code:

---

x& x::operator+(const x& xop)

---

Why are you returning a reference? An operator + is supposed to return an x object, not an x reference. Here is the correction.

Code:

---

x x::operator+(const x& xop)

---

Now look inside that function -- you're doing this:

Code:

---

    x* xreturn=new x(itmp,ptmp);  // Wrong!

---

There is no reason to use "new", and you doing this is the cause of the memory leaks. You need to remember one thing -- whatever you use to call "new", delete must be issued. So tell us -- where is the "delete" for this xreturn object? Is it in your code anywhere? No.

Here is the correction:

Code:

---

x x::operator+(const x& xop)
{
  int itmp=this->geti()+xop.geti();
  int ptmp=this->getp()+xop.getp();
  return x(temp, ptmp);
}

---

No call to new whatsoever. How does this work? An x object is created locally, and is returned since the function returns an x. You don't need "xreturn" or anything like that. When the object is returned, the copy constructor takes over and creates another object that gets sent back to the caller.

Regards,

Paul McKenzie

Quote:

---

Originally Posted by Paul McKenzie

Code:

---

x x::operator+(const x& xop)
{
  int itmp=this->geti()+xop.geti();
  int ptmp=this->getp()+xop.getp();
  return x(temp, ptmp);
}

---

---

this is a new thing for me but it is little bit different than that of general return. It does not work in returning by reference.
see it i try this before making making dynamic class in operator+ function

Code:

---

    cout<<"opearator+ start...\n";
    int itmp=this->geti()+xop.geti();
    int ptmp=this->getp()+xop.getp();
    x wert(itmp,ptmp);
    cout<<"operator+ end...\n";
    return wert;//x(itmp,ptmp);

---

It hangs in middle of execution a screen appears say class.exe has stopped working then i have to clik on close program.
reason for hang: after coming wert to main function it is temporary data so when we call operator= it makes error
What is difference in two returns.
Due this error i make return data in dynamic memory.

since any of my data is not greater than 100 so can i make this change in code to cure void x::change()

Code:

---

void x::change(int a,int b)
{
    cout<<"void change() start ..."<<endl;
    i=a;
    if(  (*p<-1000) || (*p>1000)  )
        delete p;
    else
        cout<<"class members are not initialized \a\a\a"<<endl;
    int*op=new int(b);
    p=op;
    cout<<"void change()  end  ..."<<endl;
}

---

this is not the ultimate cure but work in this code.

thanks paul for answering/helping .

I have got the solution of my own question correct code which will work fine is here

Code:

---

#define _CRTDBG_MAP_ALLOC
#include <stdlib.h>
#include <crtdbg.h>
#include<iostream>
using namespace std;
class x
{
    private:
    int i;
    int* p;

    public:
    x(int xi,int xp);
    x(const x&);
    ~x();
    void get() const;
    int geti() const ;
    int getp() const;
    x& operator=(const x&);
    x operator+(const x&);
    void change(int ,int);
        void tk()
        {
                cout<<"tk\n";
                i =1;
                p=new int(0);
        }
};
static int m(0);
x::x(int xi=1,int xp=1)
{
       
    cout<<"constructor start..."<<endl;
    i=xi;
    p=new int(xp);
        m++;
    cout<<"constructor  end..."<<m<<endl<<endl;
       
}
x::x(const x& xc)
{
    cout<<"copy constructor start ..."<<endl;
        //cout<<"going to copy in "<<i<<"  "<<*p<<endl;
    int it=xc.geti();
    int pt=xc.getp();
        this->tk();
    this->change(it,pt);
        m++;
    cout<<"copy constructor  end  ..."<<endl;
}
static int j;
x::~x()
{
       
    cout<<"destructor start..."<<endl;
    delete p;
        j++;
    cout<<"destructor  end ..."<<j<<endl<<endl;
       
}
void x::get() const
{
    cout<<"void get() start ..."<<endl;
    cout<<"*******:  "<<i<<endl;
    cout<<"pointer:  "<<*p<<endl;
    cout<<"void get()  end ... "<<endl<<endl;
}
int x::geti() const
{
    return i;
}
int x::getp() const
{
        int a=*p;
    return a;
}
void x::change(int a,int b)
{
    cout<<"void change() start ..."<<endl;
    i=a;
    delete p;
    int*op=new int(b);
    p=op;
    cout<<"void change()  end  ..."<<endl;
}
x& x::operator=(const x& xn)
{
    cout<<"operator= start..."<<endl;
    int it=xn.i;
    int pt=xn.getp();
    change(it,pt);
    cout<<"result"<<endl<<i<<"\t"<<*p<<endl;
    return *this;
}
x x::operator+(const x& xop)
{
        cout<<"opearator+ start...\n";
    int itmp=this->geti()+xop.geti();
    int ptmp=this->getp()+xop.getp();
    //x* xreturn=new x(itmp,ptmp);
        x tr(itmp,ptmp);
        cout<<"operator+ end  ...\n";
    return tr;//x(itmp,ptmp);//*xreturn;
}
int main()
{
    x x3;
        x x2(10,20);
    x x4(100,200);
    x3=(x4+x2);
    cout<<"result"<<endl;
    x3.get();
        cout<<m<<"  "<<j<<endl;
        _CrtDumpMemoryLeaks();
    return 0;
}

---

i use static variable to get information that is there any kind of memory leak. as i found number of times constructor and copy constructor called is equal to number of times destructor called but still there are memory leak reported by header and _CrtDumpMemoryLeaks(); .
Dynamic memory is also created by tk function but that is deleted by change and the memory created by change is deleted by destructor(i may wrong in this paragraph)
see what comes in my output windows

Code:

---

'class.exe': Loaded 'E:\Users\Vikash Chandola\Documents\Visual Studio 2008\Projects\class\Debug\class.exe', Symbols loaded.
'class.exe': Loaded 'E:\Windows\System32\ntdll.dll'
'class.exe': Loaded 'E:\Windows\System32\kernel32.dll'
'class.exe': Loaded 'E:\Windows\System32\KernelBase.dll'
'class.exe': Loaded 'E:\Program Files\AVAST Software\Avast\snxhk.dll'
'class.exe': Loaded 'E:\Windows\winsxs\x86_microsoft.vc90.debugcrt_1fc8b3b9a1e18e3b_9.0.30729.1_none_bb1f6aa1308c35eb\msvcp90d.dll'
'class.exe': Loaded 'E:\Windows\winsxs\x86_microsoft.vc90.debugcrt_1fc8b3b9a1e18e3b_9.0.30729.1_none_bb1f6aa1308c35eb\msvcr90d.dll'
Detected memory leaks!
Dumping objects ->
{144} normal block at 0x005149B0, 4 bytes long.
 Data: <    > DC 00 00 00
{140} normal block at 0x00514AC8, 4 bytes long.
 Data: <    > C8 00 00 00
{139} normal block at 0x00514A88, 4 bytes long.
 Data: <    > 14 00 00 00
Object dump complete.
The program '[2964] class.exe: Native' has exited with code 0 (0x0).

---

this output also not give information about the line of memory leak.
so here is only one thing remain that is memory leak where is that in program?

Quote:

---

Originally Posted by vkash

Code:

---

int main()
{
    x x3;
        x x2(10,20);
    x x4(100,200);
    x3=(x4+x2);
    cout<<"result"<<endl;
    x3.get();
        cout<<m<<"  "<<j<<endl;
        _CrtDumpMemoryLeaks();
    return 0;
}

---

---

You call _CrtDumpMemoryLeaks before the destructors of the local variables in main are called. The easiest way to prevent this is to create an additional block inside main.

Code:

---

int main()
{
    {
        x x3;
        // etc.
    }
    _CrtDumpMemoryLeaks();
}

---

That way, all local variables in the block are destroyed before _CrtDumpMemoryLeaks is called.

Quote:

---

Originally Posted by vkash

I have got the solution of my own question correct code which will work fine is here

---

All you had to do was to initialize the p pointer to NULL in the copy constructor. There is no need to introduce tk or any other member variables.

Code:

---

x::x(const x& xc) : p(0)
{
    int it=xc.geti();
    int pt=xc.getp();
    this->change(it,pt);
}

---

Also, your operator + must work for this:

Code:

---

x x::operator+(const x& xop)
{
    int itmp=this->geti()+xop.geti();
    int ptmp=this->getp()+xop.getp();
    return x(tmp, ptmp);
}

---

If it doesn't, then that means you still have bugs in your code. Changing the code around to create a temporary is not a solution -- all you're doing is moving the bug somewhere else in the code.

Regards,

Paul McKenzie`

Quote:

---

Originally Posted by vkash

Due this error i make return data in dynamic memory.

---

You need to first understand why an error exists before coding changes. All you did when you changed the code to do dynamic allocation is to make the program more wrong than it already was.

Quote:

---

since any of my data is not greater than 100 so can i make this change in code to cure void x::change()

---

Even though you changed it, the pattern that I'm seeing when you're writing code is that you come up with whatever you think is OK, but not research if it really is OK. That is not the way to learn C++.

Code:

---

void x::change(int a,int b)
{
    cout<<"void change() start ..."<<endl;
    i=a;
    if(  (*p<-1000) || (*p>1000)  )
        delete p;
    else
        cout<<"class members are not initialized \a\a\a"<<endl;
}

---

You never delete dynamic memory conditionally, while at the same time you're always unconditionally allocating it. What if I constructed the x object like this?

Code:

---

x foo(1000000, 1000000);

---

Your constructor that takes two arguments gladly accepts those arguments. Now if "change" is called, you are conditionally deleting the pointer p, which is wrong -- p was allocated in the two argument constructor. Thus you now have a memory leak.

Regards,

Paul McKenzie

Reply to paul

Quote:

---

Originally Posted by Paul McKenzie

All you had to do was to initialize the p pointer to NULL in the copy constructor. There is no need to introduce tk or any other member variables.

Code:

---

x::x(const x& xc) : p(0)
{
    int it=xc.geti();
    int pt=xc.getp();
    this->change(it,pt);
}

---

---

this is good idea i forget to use such things next time i will use it if required.

Quote:

---

Originally Posted by Paul McKenzie

Also, your operator + must work for this:

Code:

---

x x::operator+(const x& xop)
{
    int itmp=this->geti()+xop.geti();
    int ptmp=this->getp()+xop.getp();
    return x(tmp, ptmp);
}

---

---

yeah it works fine. i try to know why other are not working that's why i try others.

Quote:

---

Originally Posted by Paul McKenzie

You need to first understand why an error exists before coding changes. All you did when you changed the code to do dynamic allocation is to make the program more wrong than it already was.

---

I know why it was wrong there when i return a class x data from operator+ to main then it is temporary just as it comes to main it is sent to operator= is called which is taking a temporary data this makes error(in some cases wrong deletion of data). other sources of error are deleting that memory which is even not initialized that's why i put tk function(to initialize if not initialized) .

Quote:

---

Originally Posted by Paul McKenzie

Even though you changed it, the pattern that I'm seeing when you're writing code is that you come up with whatever you think is OK, but not research if it really is OK. That is not the way to learn C++.

---

you are also right here. i make these cross cuts(i mean wrong way of removing bugs) in program because i did not found any other idea. I work in this code for 5 hours even after that this code makes a lot error(hang) then making cross cuts(as i did) is only idea come in mind. next time i will try to not use such bad methods(as i use in this like dynamic memory).


we can change constructor to not take value greater than 100 less than 0

Code:

---

x::x(int xi=1,int xp=1)
{
        bool d(0);
        if((xi>100)||(xp>100)||(xi<0))||(xp<0))
        {
                cout<<"setting by default large data \n";
        d=1
        }
       
    cout<<"constructor start..."<<endl;
    i=xi;
    p=new int(xp);
        if(d=1)
        {
                i=1;
                p=new int(1);
        }
        m++;
    cout<<"constructor  end..."<<m<<endl<<endl;
       
}
//i did not test it

---

I think this time constructor is made correctly. If execution enter first if then middle part is stuff but i don't know how to still let that to not work in this case one can make other function which called if d=0,d=1 one time one will b executed but it will increase code length(making them inline will boost execution i think so).
Now i think all bugs are fixed with your copy constructor and constructor that i propose. In all thing i write here i may wrong so (as you do usually) tell me if i am wrong.
special thanks for telling me that i am moving in wrong path of study.


reply to D_Drmmr
thanks for this idea it works. I was searching for a tool to detect memory leak since last month. S_M_A and you give me this tool. thanks to both guys.

vkash, read the entire thread. There's no need to call _CrtDumpMemoryLeaks(). The easiest way that I've found to enable CRT memory leak detection is as described in my post #12 of that thread.

In post #31 you also find an experimental project made in an attempt to solve John E's issue. If you use that project as an example the memleaks show up with source line references and you just have to double-click that line or press F4 to jump from place entry to entry.

Quote:

---

Originally Posted by vkash

I know why it was wrong there when i return a class x data from operator+ to main then it is temporary just as it comes to main it is sent to operator= is called which is taking a temporary data this makes error(in some cases wrong deletion of data). other sources of error are deleting that memory which is even not initialized that's why i put tk function(to initialize if not initialized) .

---

I think my explanation is not fully correct (copy constructor copy data).


why returning by reference not work

Code:

---

#define _CRTDBG_MAP_ALLOC
#include <stdlib.h>
#include <crtdbg.h>
#include<iostream>
using namespace std;
class x
{
    private:
    int i;
    int* p;

    public:
    x(int xi,int xp);
    x(const x&);
    ~x();
    void get() const;
    int geti() const ;
    int getp() const;
    x& operator=(const x&);
    x& operator+(const x&);
    void change(int ,int);
        void tk()
        {
                cout<<"tk\n";
                i =1;
                p=new int(0);
        }
};
static int m(0);
x::x(int xi=1,int xp=1)
{
        cout<<"constructor start..."<<xi<<endl;
        bool d(0);
        if((xi>1000)||(xp>1000)||(xi<0)||(xp<0))
        {
                cout<<"setting by default large data"<<xi<<xp<<" \n";
        d=1;
        }
       
   
    i=xi;
    p=new int(xp);
        if(d==1)
        {
                i=1;
                p=new int(1);
        }
        m++;
    cout<<"constructor  end..."<<m<<endl<<endl;
       
}
x::x(const x& xc)
{
    cout<<"copy constructor start ..."<<endl;
        //cout<<"going to copy in "<<i<<"  "<<*p<<endl;
        p=0;
    int it=xc.geti();
    int pt=xc.getp();
    this->change(it,pt);
        m++;
    cout<<"copy constructor  end  ..."<<endl;
}
static int j;
x::~x()
{
       
    cout<<"destructor start..."<<i<<endl;
    delete p;
        j++;
    cout<<"destructor  end ..."<<j<<endl<<endl;
       
}
void x::get() const
{
    cout<<"void get() start ..."<<endl;
    cout<<"*******:  "<<i<<endl;
    cout<<"pointer:  "<<*p<<endl;
    cout<<"void get()  end ... "<<endl<<endl;
}
int x::geti() const
{
    return i;
}
int x::getp() const
{
        int a=*p;
    return a;
}
void x::change(int a,int b)
{
    cout<<"void change() start ..."<<endl;
    i=a;
    delete p;
    int*op=new int(b);
    p=op;
    cout<<"void change()  end  ..."<<endl;
}
x& x::operator=(const x& xn)
{
    cout<<"operator= start..."<<endl;
    int it=xn.i;
    int pt=xn.getp();
    change(it,pt);
    cout<<"operator= end ..."<<endl;
    return *this;
}
x& x::operator+(const x& xop)  //here returning by reference not work
{
        cout<<"opearator+ start...\n";
    int itmp=this->geti()+xop.geti();
    int ptmp=this->getp()+xop.getp();
    //x* xreturn=new x(itmp,ptmp);
        x tr(itmp,ptmp);
        cout<<"operator+ end  ..."<<"\n";
    return tr;//x(itmp,ptmp);//*xreturn;
}
int main()
{
        {
    x x3;
        x x2(10,20);
    x x4(100,200);
    x3=(x4+x2);
    cout<<"result"<<endl;
    x3.get();
        }
        cout<<m<<"  "<<j<<endl;
        _CrtDumpMemoryLeaks();
        return 0;

}

---

It hangs in between execution.
I can't explain why returning by reference not work. Can you please explain?

Quote:

---

Originally Posted by vkash

I can't explain why returning by reference not work. Can you please explain?

---

Because you return a reference to a local variable. As soon as you leave the function, that variable is destroyed and you are left with a dead reference.

Quote:

---

Originally Posted by vkash

why returning by reference not work

---

It is undefined behavior to return a reference or pointer to a local variable.

Look at my code -- what is being returned is an x object, not a reference. Look at this very simple example:

Code:

---

int foo()
{
    int i = 10;
    return i;  // ok -- returning the value of i
}

---

Compare it with this:

Code:

---

int &foo()
{
  int i = 10;
  return i;  // undefined behavior -- trying to return the variable i as reference
}

---

The functions above are fundamentally no different than the operator + you coded, and the operator + that I coded. Do you understand the fundamental difference between the two functions?

The first one takes the value of i, and returns it as an int. The second takes the variable i and returns a reference to that variable. That variable goes away when the function returns, since it is local to that function. So the difference is between returning a value and returning a variable reference or pointer.

In your class, you are returning a reference to a variable that no longer exists. In the code I wrote previously, an x object's value is returned as an x.

Regards,

Paul McKenzie

see this code it is implementation of operator= for a class that has pointer to char(code is from book).mainly consider highlighted block.

Code:

---


#include <iostream>
#include <cstring>
using std::cout;
using std::endl;

class CMessage
{
  private:
    char* pmessage;                    // Pointer to object text string

  public:
    // Function to display a message
    void ShowIt() const
    {
      cout << endl << pmessage;
    }

    //Function to reset a message to *
    void Reset()
    {
      char* temp = pmessage;
      while(*temp)
        *(temp++) = '*';
    }

  // Overloaded assignment operator for CMessage objects
    CMessage& operator=(const CMessage& aMess)
    {
      if(this == &aMess)              // Check addresses, if equal
        return *this;                  // return the 1st operand

      // Release memory for 1st operand
      delete[] pmessage;
      pmessage = new char[strlen(aMess.pmessage) + 1];

      // Copy 2nd operand string to 1st
      strcpy_s(this->pmessage, strlen(aMess.pmessage) + 1, aMess.pmessage);

      // Return a reference to 1st operand
      return *this;
    }

    // Constructor definition
    CMessage(const char* text = "Default message")
    {
      pmessage = new char[strlen(text) + 1];      // Allocate space for text
      strcpy_s(pmessage, strlen(text)+1, text);    // Copy text to new memory
    }

    // Copy constructor definition
    CMessage(const CMessage& aMess)
    {
      size_t len = strlen(aMess.pmessage)+1;
      pmessage = new char[len];
      strcpy_s(pmessage, len, aMess.pmessage);
    }

    // Destructor to free memory allocated by new
    ~CMessage()
    {
      cout << "Destructor called."    // Just to track what happens
          << endl;
      delete[] pmessage;              // Free memory assigned to pointer
    }
};

int main()
{
  CMessage motto1("The devil takes care of his own");
  CMessage motto2;

  cout << "motto2 contains - ";
  motto2.ShowIt();
  cout << endl;

  motto2 = motto1;                      // Use new assignment operator

  cout << "motto2 contains - ";
  motto2.ShowIt();
  cout << endl;

  motto1.Reset();                      // Setting motto1 to * doesn't
                                      // affect motto2

  cout << "motto1 now contains - ";
  motto1.ShowIt();
  cout << endl;

  cout << "motto2 still contains - ";
  motto2.ShowIt();
  cout << endl;

  return 0;
}

---

operator= return a reference to a class variable then why this works?

this works because unlike my last code here there is no variable whose scope is inside operator=. In my code itmp and[ptmp has scope only inside the operator+ braces and returning class has that data. Returning by value(not by reference) work because in that case there is copy constructor which make another copy before destructing.

moral never return any data by any way which is somehow related to data inside function braces.
Is my explanation correct.


but i am still in little confusion
firstly=> for the above what happen when i write (motto1=motto2)=motto3 this (motto1=motto2) is called first the returning data is reference to what?? even now operator= will called once again with returned data motto3. In this returned data( after call motto1 and motto2) is used to call once again is it existing even now if yes then what is it's scope and when it will destructed. In the book it is written if we return here by value (in first call of motto1 and motto2) then there will an error. I think error should in both case but it is not happening in case of reference.(even after all these. this code is working is fine)

secondly => since above code is working in case when we return by reference a data that is not dependent on any other data inside function body then if we make itmp and ptmp global variable then it should work but not working even in that case. Why?
If returning is done by this method return x(itmp,ptmp) then this run but wrong data appear.
I think because return data destructor is called before as it moves to main.

Quote:

---

Originally Posted by vkash

see this code it is implementation of operator= for a class that has pointer to char(code is from book).mainly consider highlighted block.

---

There is a flaw in the operator =. Can you spot it?

Code:

---

// Overloaded assignment operator for CMessage objects
    CMessage& operator=(const CMessage& aMess)
    {
      if(this == &aMess)              // Check addresses, if equal
        return *this;                  // return the 1st operand

      // Release memory for 1st operand
      delete[] pmessage;
      pmessage = new char[strlen(aMess.pmessage) + 1];

      // Copy 2nd operand string to 1st
      strcpy_s(this->pmessage, strlen(aMess.pmessage) + 1, aMess.pmessage);

      // Return a reference to 1st operand
      return *this;
    }

---

Note the code in red. Why is this flawed?

Regards,

Paul McKenzie

Quote:

---

Originally Posted by Paul McKenzie

There is a flaw in the operator =. Can you spot it?

Code:

---

// Overloaded assignment operator for CMessage objects
    CMessage& operator=(const CMessage& aMess)
    {
      if(this == &aMess)              // Check addresses, if equal
        return *this;                  // return the 1st operand

      // Release memory for 1st operand
      delete[] pmessage;
      pmessage = new char[strlen(aMess.pmessage) + 1];

      // Copy 2nd operand string to 1st
      strcpy_s(this->pmessage, strlen(aMess.pmessage) + 1, aMess.pmessage);

      // Return a reference to 1st operand
      return *this;
    }

---

Note the code in red. Why is this flawed?

Regards,

Paul McKenzie

---

what's that flaw

Quote:

---

Originally Posted by vkash

what's that flaw

---

What would happen if new[] fails on the next line?

Code:

---

pmessage = new char[strlen(aMess.pmessage) + 1];

---

When new fails, a std::bad_alloc exception is thrown. What happened to the original string? You've already destroyed it, and your object is now in an inconsistant state.

So how do you fix it?

Regards,

Paul McKenzie

Quote:

---

Originally Posted by Paul McKenzie

What would happen if new[] fails on the next line?

Code:

---

pmessage = new char[strlen(aMess.pmessage) + 1];

---

When new fails, a std::bad_alloc exception is thrown. What happened to the original string? You've already destroyed it, and your object is now in an inconsistant state.

So how do you fix it?

Regards,

Paul McKenzie

---

Oh that is it.
this code will work.

Code:

---

// Overloaded assignment operator for CMessage objects
    CMessage& operator=(const CMessage& aMess)
    {
      try
{
      if(this == &aMess)              // Check addresses, if equal
        return *this;                  // return the 1st operand

      // Release memory for 1st operand
      delete[] pmessage;
      pmessage = new char[strlen(aMess.pmessage) + 1];

      // Copy 2nd operand string to 1st
      strcpy_s(this->pmessage, strlen(aMess.pmessage) + 1, aMess.pmessage);

      // Return a reference to 1st operand
      return *this;
}
catch(bad_alloc &ex)
  {
    cout << "Memory allocation failed." << endl
        << "The information from the exception object is: "
        << ex.what() << endl;
cout<<"closing";
exit(0)
  }
}

---

here this bad_alloc will called if strlen did not work properly. if that can't give information about what is length of string. this will close program if bad_alloc send.
am i correct.
if yes then help me in other questions

Quote:

---

Originally Posted by vkash

but i am still in little confusion
firstly=> for the above what happen when i write (motto1=motto2)=motto3 this (motto1=motto2) is called first the returning data is reference to what?? even now operator= will called once again with returned data motto3. In this returned data( after call motto1 and motto2) is used to call once again is it existing even now if yes then what is it's scope and when it will destructed. In the book it is written if we return here by value (in first call of motto1 and motto2) then there will an error. I think error should in both case but it is not happening in case of reference.(even after all these. this code is working is fine)

secondly => since above code is working in case when we return by reference a data that is not dependent on any other data inside function body then if we make itmp and ptmp global variable then it should work but not working even in that case. Why?
If returning is done by this method return x(itmp,ptmp) then this run but wrong data appear.
I think because return data destructor is called before as it moves to main.

---

see my second last post.

Calling exit just because a function fails is extremely poor design. You should never do that. Better just to let your function throw an exception and let the caller decide what to do with it.

Quote:

---

Originally Posted by GCDEF

Calling exit just because a function fails is extremely poor design. You should never do that. Better just to let your function throw an exception and let the caller decide what to do with it.

---

is it better

Code:

---

// Overloaded assignment operator for CMessage objects
    CMessage& operator=(const CMessage& aMess)
    {
      try
{
      if(this == &aMess)              // Check addresses, if equal
        return *this;                  // return the 1st operand

      // Release memory for 1st operand
      delete[] pmessage;
      pmessage = new char[strlen(aMess.pmessage) + 1];

      // Copy 2nd operand string to 1st
      strcpy_s(this->pmessage, strlen(aMess.pmessage) + 1, aMess.pmessage);

      // Return a reference to 1st operand
      return *this;
}
catch(bad_alloc &ex)
  {
    cout << "Memory allocation failed." << endl
        << "The information from the exception object is: "
        << ex.what() << endl;
cout<<"seting by default\n";
pmessage = new char[30];
char array[30]="default message";
strcpy_s(this->pmessage, strlen(array) + 1, array);
  }
}

---

i think this will not make any error in compiling not tested.Is it Ok.
thanks for advise.
please answer question i post in my third last post.

Quote:

---

Originally Posted by vkash

but i am still in little confusion
firstly=> for the above what happen when i write (motto1=motto2)=motto3 this (motto1=motto2) is called first the returning data is reference to what?? even now operator= will called once again with returned data motto3. In this returned data( after call motto1 and motto2) is used to call once again is it existing even now if yes then what is it's scope and when it will destructed. In the book it is written if we return here by value (in first call of motto1 and motto2) then there will an error. I think error should in both case but it is not happening in case of reference.(even after all these. this code is working is fine)

secondly => since above code is working in case when we return by reference a data that is not dependent on any other data inside function body then if we make itmp and ptmp global variable then it should work but not working even in that case. Why?
If returning is done by this method return x(itmp,ptmp) then this run but wrong data appear.
I think because return data destructor is called before as it moves to main

---

Quote:

---

Originally Posted by vkash

is it better

Code:

---

// Overloaded assignment operator for CMessage objects
    CMessage& operator=(const CMessage& aMess)
    {
      try
{
      if(this == &aMess)              // Check addresses, if equal
        return *this;                  // return the 1st operand

      // Release memory for 1st operand
      delete[] pmessage;
      pmessage = new char[strlen(aMess.pmessage) + 1];

      // Copy 2nd operand string to 1st
      strcpy_s(this->pmessage, strlen(aMess.pmessage) + 1, aMess.pmessage);

      // Return a reference to 1st operand
      return *this;
}
catch(bad_alloc &ex)
  {
    cout << "Memory allocation failed." << endl
        << "The information from the exception object is: "
        << ex.what() << endl;
cout<<"seting by default\n";
pmessage = new char[30];
char array[30]="default message";
strcpy_s(this->pmessage, strlen(array) + 1, array);
  }
}

---

i think this will not make any error in compiling not tested.Is it Ok.
thanks for advise.
please answer question i post in my third last post.

---

No, absolutely not. There's no reason for couts in that function either. Again, let your function throw the exception and let the caller decide what to do with it.

It's really unlikely new would fail, but Paul's point was that if it does, the object should remain unchanged.

Quote:

---

Originally Posted by GCDEF

No, absolutely not. There's no reason for couts in that function either. Again, let your function throw the exception and let the caller decide what to do with it.

It's really unlikely new would fail, but Paul's point was that if it does, the object should remain unchanged.

---

I don't understand what you say in first paragraph.
but if you want that catch should do nothing just keep it's original value then it is more simpler see it

Code:

---

// Overloaded assignment operator for CMessage objects
    CMessage& operator=(const CMessage& aMess)
    {
char arr[](pmessage); //will it work error may pointer to char conversion invalid
      try
{
      if(this == &aMess)              // Check addresses, if equal
        return *this;                  // return the 1st operand

      // Release memory for 1st operand
      delete[] pmessage;
      pmessage = new char[strlen(aMess.pmessage) + 1];
   
      // Copy 2nd operand string to 1st
      strcpy_s(this->pmessage, strlen(aMess.pmessage) + 1, aMess.pmessage);

      // Return a reference to 1st operand
      return *this;
}
catch(bad_alloc &ex)
  {
    cout << "Memory allocation failed." << endl
        << "The information from the exception object is: "
        << ex.what() << endl;
cout<<"making no change\n";
pmessage=new char[strlen(arr)+1];
strcpy_s(pmessage,strlen(arr)+1),arr);
  }
}

---

what about this.

Quote:

---

Originally Posted by vkash

philip I have no intension to show up i put real code but "int ig ers"(remove space) word s appearing as ******* I don't know why this happening.
(Type int ig er(without space ) in any post you will see that it appear as ******* it's not my fault.)

---

I have no idea why the board feels the need to censor that misspelling. However, it will certainly not censor the correct spelling, "integers".

Quote:

---

Originally Posted by vkash

Oh that is it.
this code will work.

---

No. What if I am using this class want to keep the object intact and not have the message deleted on me?

Second, what happens when CMessage is destroyed? You will then be deleting a pointer to an invalid object.

The solution is simple -- you don't need try/catch:

Code:

---

CMessage& operator=(const CMessage& aMess)
{
      if(this == &aMess)              // Check addresses, if equal
        return *this;                  // return the 1st operand

      char *temp =  new char[strlen(aMess.pmessage) + 1];
      strcpy_s(temp, strlen(aMess.pmessage) + 1, aMess.pmessage);
      delete [] pMessage;
      pMessage = temp;
      return *this;
}

---

A temporary is created first. If new[] throws an exception, the pMessage is still intact, and the object has not changed. If new[] doesn't throw an exception, then temp is populated with the data, and then pMessage is deleted and assigned the value of temp.

Regards,

Paul McKenzie

Quote:

---

Originally Posted by vkash

I don't understand what you say in first paragraph.

---

Let the user handle the exception, not your class. All you need to do is document that the assignment can throw a std::bad_alloc exception. Your code shouldn't be catching anything.

Quote:

---

but if you want that catch should do nothing just keep it's original value then it is more simpler see it

---

That is no solution -- it is not simple, and can lead to another exception being thrown, causing the program to terminate. If new[] threw an exception originally, how can you call new[] again to create the string inside the catch block?

Regards,

Paul McKenzie

Quote:

---

Originally Posted by vkash

I don't understand what you say in first paragraph.
but if you want that catch should do nothing just keep it's original value then it is more simpler see it

Code:

---

// Overloaded assignment operator for CMessage objects
    CMessage& operator=(const CMessage& aMess)
    {
char arr[](pmessage); //will it work error may pointer to char conversion invalid
      try
{
      if(this == &aMess)              // Check addresses, if equal
        return *this;                  // return the 1st operand

      // Release memory for 1st operand
      delete[] pmessage;
      pmessage = new char[strlen(aMess.pmessage) + 1];
   
      // Copy 2nd operand string to 1st
      strcpy_s(this->pmessage, strlen(aMess.pmessage) + 1, aMess.pmessage);

      // Return a reference to 1st operand
      return *this;
}
catch(bad_alloc &ex)
  {
    cout << "Memory allocation failed." << endl
        << "The information from the exception object is: "
        << ex.what() << endl;
cout<<"making no change\n";
pmessage=new char[strlen(arr)+1];
strcpy_s(pmessage,strlen(arr)+1),arr);
  }
}

---

what about this.

---

One more time, in addition to Paul's comments, an = operator shouldn't contain any cout statements. Typically the user wouldn't want to see them or have any idea what to do about it. When something goes wrong, it should be up to the caller of that function to determine how to handle the error. All your function should do is indicate somehow to its caller that something has gone wrong, and provide a way for the caller to determine what. Exceptions are designed to provide just such a mechanism.

And to add to what GCDEF mentions, if you wanted a catch block, the only viable thing to do is rethrow the std::bad_alloc back to the caller.

You shouldn't be attempting to create more dynamic strings (this is what caused the exception to begin with), and you shouldn't be issuing cout statements or calling exit(0).

As to the exit(0), to emphasize what was already stated -- what if I need to close files, handles, shut down a socket, or some other cleanup routine, but I can't because your CMessage object stops the application dead by calling exit(0)? You would receive huge complaints from persons using your class.

Regards,

Paul McKenzie

thanks to paul and GCDEF for giving me information about exception and how to use them.
BUT I have still problem when data is returned by reference I have studied both return by reference and return by value(a month ago) usually return by value is used. Can you please refresh my memory on what is difference in these two returning methods.

Quote:

---

Originally Posted by vkash

thanks to paul and GCDEF for giving me information about exception and how to use them.
BUT I have still problem when data is returned by reference I have studied both return by reference and return by value(a month ago) usually return by value is used. Can you please refresh my memory on what is difference in these two returning methods.

---

If you return a reference to something, it has to exist after the function exits. That's why you can't return a reference to a variable declared in the function because it goes out of scope when the function ends.

Paul explained it in post 18

http://www.codeguru.com/forum/showpo...9&postcount=18

Quote:

---

Originally Posted by vkash

moral never return any data by any way which is somehow related to data inside function braces.
Is my explanation correct.

---

The term 'data' is not accurate in this discussion. You should talk about 'objects', because objects have a lifetime associated with them (they are created and destroyed at some time during program execution), whereas data has not (it may linger in memory). A pointer or reference to an object has no knowledge of the object's lifetime. Thus you can have a reference to an object that no longer exists.

Quote:

---

Originally Posted by vkash

but i am still in little confusion
firstly=> for the above what happen when i write (motto1=motto2)=motto3 this (motto1=motto2) is called first the returning data is reference to what?? even now operator= will called once again with returned data motto3. In this returned data( after call motto1 and motto2) is used to call once again is it existing even now if yes then what is it's scope and when it will destructed. In the book it is written if we return here by value (in first call of motto1 and motto2) then there will an error. I think error should in both case but it is not happening in case of reference.(even after all these. this code is working is fine)

---

When you implement an overloaded operator, a copy constructor or an assignment operator, you should implement them in such a way that they work in a similar way as the same operations applied to built-in types. This is not mandated by the standard, but if you deviate from this, your code will be hard to use, because it has subtle differences from what the average programmer (and therefore other code) would expect. As an example of how important this is: std::auto_ptr's copy constructor and assignment operator do not behave like other types, which has caused it to become deprecated in the new standard (also, because there is a better alternative now).

What this means in practice is that your overloaded operators should use a specific prototype, e.g. the binary + operator should return a copy, an assignment operator should return a reference to itself, etc. In code:

Code:

---

class X
{
public:
    X(const X&);
    X& operator =(const X&);
    X& operator +=(const X&);
    X operator +(const X&) const; // or you can define this as a non-member function taking two const X&'s.
};

---

So, to understand the code you posted, you should think about what this code does:

Code:

---

int x = 1, y = 2, z = 3;
(x = y) = z;

---

What is the value of x, y and z after the second statement?
When you use your own class instead of an int, the behavior should be the same. So, then the question becomes: how should the assignment operator of your own class be implemented to achieve that?

I understood it
at the end of operator+ a variable X of type is returned that is destroyed latter but returning by reference work in case of operator= because here a reference to object which is transfered to it at calling not created in operator=. that's thing i want to know. thanks guys.

One more thing is (x=y)=z as stated by D_Drmmr here if x y z are objects then x=y is done first now x and y are same. a reference to x is returned then x = z called thus all happen correctly.(now all three are equal) that's why operator= works without hang in both code.
BIG thanks to all persons for helping now i think my problems is solved.

Quote:

---

Originally Posted by vkash

One more thing is (x=y)=z as stated by D_Drmmr here if x y z are objects then x=y is done first now x and y are same. a reference to x is returned then x = z called thus all happen correctly.(now all three are equal) that's why operator= works without hang in both code.

---

I suggest you learn to check your results. Your understanding seems to be in the right direction, but your conclusion is wrong. If you take the time to write a small program and check the result, you'll have a lot smaller chance of misunderstanding.