need little help of bug fixers.

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