overloading operator+ (C++)

Question

Hey so I am working on an overload of +, but when I try to do a simple call like

statistician a,b,c;

a = b+c;

When I do the above call it crashes. and if i do an overload of =, it just returns a zero. here is some of my code. Thanks for the help guys!!!

FYI next_number(double value), increases the dynamic array by 1, and puts that value in end of the array.

statistician  operator+(const statistician& left, const statistician& right) 
{
    statistician temp;

    if(left.m_iArraySize == 0)
    {
        return right;
    }
    else if(right.m_iArraySize == 0)
    {
        return left;
    }
    else
    {
        statistician temp;

        for(int i =0; i< left.m_iArraySize; i++)
        {
            temp.next_number(left.m_dSeqArray[i]);
        }

        for(int i =0; i< right.m_iArraySize; i++)
        {
            temp.next_number(right.m_dSeqArray[i]);
        }
        return temp;
    }

} 

The Implementation of the class

#include "Statistician.h"
#include <iostream>
using namespace std;


namespace main_savitch_2C
{

statistician::statistician()
{
    m_iArraySize=0;
    m_dSeqArray = new double[1]; // Preset our list to 1 items
    m_dSeqArray[0] = 0; 

}



statistician::~statistician()
{    
    delete[] m_dSeqArray;
    m_iArraySize=0;

}

statistician::statistician(const statistician &s)
{
    m_iArraySize=0;
    m_dSeqArray = new double[1]; // Preset our list to 1 items  
    m_dSeqArray[0] = 0;

}  

int statistician::ChangeArraySize(int iArraySize)
{


    double *iTempArray;
    iTempArray = new double[m_iArraySize];

    //Assert Data   
    assert(iArraySize >-1);
    // Copy the info of the array into a temp array

    for(int i = 0; i < m_iArraySize-1; i++)
    {

        iTempArray[i] = m_dSeqArray[i];
    }
//    iTempArray = m_dSeqArray;
    // delete the array 

    delete[] m_dSeqArray;



    m_dSeqArray = new double[iArraySize];



    // Copy the info of the temp array  back into the orginal array variable

    for(int i = 0; i < m_iArraySize; i++)
    {

        m_dSeqArray[i] = iTempArray[i];
    }
    m_dSeqArray[iArraySize-1]=0;

    // Free our TempArray
    delete[] iTempArray;

    return 0;
}

//length
int statistician::length() const
{
    if(m_iArraySize == 0) {return 0;}

    return m_iArraySize;
}

//sum()
double statistician::sum() const
{
   if(m_iArraySize == 0) {return 0;}     

    long double dSum=0; 
    for(int i = 0; i < m_iArraySize; i++)
    {
        dSum = dSum + m_dSeqArray[i];
    }
    return dSum;
}

//mean()
double statistician::mean() const
{ 
    if(m_iArraySize == 0) {return 0;}

    double dSum,dMean; 
    int iCounter; 

    dSum=0;
    dMean=0;
    iCounter=0;

    for(int i = 0; i < m_iArraySize; i++)
    {

        dSum = dSum + m_dSeqArray[i];
        iCounter++;
    }

    // Check for Dividing by zero 
    if(dSum == 0)
    {
        dMean = 0; 
    }
    else 
    {
        dMean = dSum/iCounter; 
    }
    return dMean;

}

//reset()
int statistician::reset()
{

    delete[] m_dSeqArray;


    m_iArraySize = 0;

    m_dSeqArray = new double[1];
    m_dSeqArray[0] = 0;

    return 0;
}

//next_number(i)
int statistician::next_number(double iValue)
{
    // Ensure that size of the array has not somehow become negative 


    m_iArraySize++;

    if(m_iArraySize > 1)
    {

        ChangeArraySize(m_iArraySize);
        m_dSeqArray[m_iArraySize-1] = iValue;



    }
    else 
    {

        m_dSeqArray[m_iArraySize-1] = iValue;


    }



    return 0;
}

//next_number(int iValue)
int statistician::next_number(int iValue)
{


    m_iArraySize++;

    if(m_iArraySize > 1)
    {

        ChangeArraySize(m_iArraySize);
        m_dSeqArray[m_iArraySize-1] = iValue;



    }
    else 
    {

        m_dSeqArray[m_iArraySize-1] = iValue;


    }

    return 0;
}

//next_number(char iValue) in case a character is passed 
int statistician::next_number(char iValue)
{
     cout << "Invalid value passed to next_number!" <<endl;

    return 0;
}

//Minimum


double statistician::minimum() const
{
 if(m_iArraySize == 0) {return 0;}

    double dMinimum = m_dSeqArray[0];
    for(int i = 0; i < m_iArraySize; i++)
    {

        if(m_dSeqArray[i] < dMinimum)
        {
            dMinimum = m_dSeqArray[i];

        }     
    }
    return dMinimum;
}

//Maximum
double statistician::maximum() const
{
    if(m_iArraySize == 0) {return 0;}

    double dMaximum = m_dSeqArray[0];
    for(int i = 0; i < m_iArraySize; i++)
    {

        if(m_dSeqArray[i] > dMaximum)
        {
            dMaximum = m_dSeqArray[i];

        }     
    }
    return dMaximum;

}

//Operator Overloading
/*
statistician statistician::operator=( statistician& left)
{
    cout << left.sum();
    cout << left.length();
    cout << this->sum();

    if(this != &left)
    {
        this->reset();
        for (int i =0; i < left.m_iArraySize; i++)
        {
            this->next_number(left.m_dSeqArray[i]);
            cout<<this->next_number(left.m_dSeqArray[i]) <<endl;
            cout<<left.next_number(left.m_dSeqArray[i]) <<endl;
        }
    }
    return *this;


} */
statistician  operator+(const statistician& left, const statistician& right)
{
    statistician temp;

    if(left.m_iArraySize == 0)
    {
        return right;
    }
    else if(right.m_iArraySize == 0)
    {
        return left;
    }
    else
    {
        statistician temp;

        for(int i =0; i< left.m_iArraySize; i++)
        {
            temp.next_number(left.m_dSeqArray[i]);
        }

        for(int i =0; i< right.m_iArraySize; i++)
        {
            temp.next_number(right.m_dSeqArray[i]);
        }
        return temp;
    }

} 


bool  operator==( const statistician& left,const   statistician& right)
{ 
    cout << "L length = : "<<left.length() <<" R Length: "<< right.length()<<endl;
    if(left.length() != right.length()) { return false; }

        cout << "L minimum = : "<<left.minimum() <<" R minimum: "<< right.minimum()<<endl;
    if(left.minimum() != right.minimum()) { return false; }

    cout << "L maximum = : "<<left.maximum() <<" R maximum: "<< right.maximum()<<endl;
    if(left.maximum() != right.maximum()) { return false; }

    cout << "L sum = : "<<left.sum() <<" R sum: "<< right.sum()<<endl;
    if(left.sum() != right.sum() ) {return false; }

    cout << "L mean = : "<<left.mean() <<" R mean: "<< right.mean()<<endl;
    if(left.mean() != right.mean() ) {return false; }  






    return true;

}


} // End of Namespace 

and the .h....

#ifndef StatClass
#define StatClass
#include <cassert>


namespace main_savitch_2C
{



class  statistician     //With copy constructor
{
  public:

    // Constructor
    statistician();


    //Deconstructor 
    ~statistician();

    //Copy constructor      
    statistician(const statistician &s);

    //length
    int length() const;

    //sum()
    double sum() const;

    //mean()
    double mean() const;

    //reset()
    int reset();

    //next_number(i)
    int next_number(double iValue);
    int next_number(int iValue);
    int next_number(char iValue);

    //Minimum

    double minimum() const;

    //Maximum
    double maximum() const;


    //Operator Overloading
    //statistician operator=(statistician& left);
    friend statistician operator+(const statistician& left, const statistician& right);
    friend bool operator==(const statistician& left, const statistician& right); 





  private:
    // Private Variables
    double *m_dSeqArray;
    int m_iArraySize;

    // Private Functions
    int ChangeArraySize(int iArraySize);





};

};  
#endif

Answer 1

What does your constructor look like? Does statistician a,b,c actually initialize the array?

Answer 2

First, the canonical operator+ should be implemented in terms of operator+=, and it is in this operator that you should write your logic.

Second, statistician copy constructor does not copy anything, it just creates a default statistician. Since operator+ uses the copy constructor for its return value, you will never get what you intend.

Third, statistician::next_number() does not do what you think. You never increase the size of the internal array, so you will end up going out of bounds. This leads to undefined behaviour, probably crashing when you overwrite memory belonging to other objects.

I have not checked all the code for all methods, but you should definitely fix these three errors before continuing. Well, the first one is not so important, but the other two ones are a must.

Answer 3

If you don't define the assignment operator (operator=), then the compiler provides a default version, which does a shallow copy-- that is, it copies the pointer m_dSeqArray, but not the thing it points to. So when you use it, you wind up with two pointers pointing to the same thing. When the two statisticians pass out of scope, each calls the destructor, which tries to delete the array twice, BOOM!

Your version of the assignment operator copies each element twice:

this->next_number(left.m_dSeqArray[i]);
cout<<this->next_number(left.m_dSeqArray[i]) <<endl;

So the resultant statistician should look weird, but not be empty (unless the statistician being assigned was empty, which it is in your example).

Answer 4

Though not a direct reason for crash, I can't help pointing it out

The copy constructor looks to be a cut-paste of the constructor. This may not be right I believe. Copy constructor may be involved since operator+ involves returning a copy which could call copy constructor if it is not elided away.

How are you using 's' in the copy constructor?

statistician::statistician(const statistician &s) 
{ 
    m_iArraySize=0; 
    m_dSeqArray = new double[1]; // Preset our list to 1 items   
    m_dSeqArray[0] = 0; 
}