Question

My class NRRanNormal represents a normally-distributed random variable. By default, instances are normally distributed with mean 0 and stdev 1 (i.e., a standard normal random variable).

Sometimes when I copy NRRanNormal objects, the mean and stdev of the object copied into (or constructed via the copy constructor) are garbled and nonsense. I'm having a hard time finding the cause of this garbling.

For testing purposes, the following function displays the mean and stdev of a given NRRanNormal object:

void go(NRRanNormal& rv, const string label) {
    std::cout << label     << "\n"
              << "Mean:  " << rv.getMean()  << "\n"
              << "Stdev: " << rv.getStdev() << "\n\n";
}

Now, let's see what happens in the following 4 cases:

NRRanNormal foo;
go(foo, "foo");

NRRanNormal bar1 = foo;
go(bar1, "bar1");

NRRanNormal bar2;
bar2 = foo;
go(bar2, "bar2");

NRRanNormal bar3(foo);
go(bar3, "bar3");

The output of the above statements is the following:

foo
Mean:  0
Stdev: 1

bar1
Mean:  5.55633e-317
Stdev: 6.95332e-310

bar2
Mean:  0
Stdev: 1

bar3
Mean:  0
Stdev: 0

As you can see, simply instantiating an object (foo) works as expected.

Now, when I do NRRanNormal bar1 = foo;, the object bar1 is garbled. However, when I do NRRanNormal bar2; bar2 = foo;, the object bar2 is not garbled. This puzzles me. I thought that a statement block such as

MyClass A;
MyClass B = A;

is actually converted by the compiler to the statement block

MyClass A;
MyClass B;
B = A;

Therefore, unless what I just wrote immediately above is incorrect, it seems that bar1 and bar2 should have exactly the same member values. But as you can see from the output pasted above, bar1 is garbled whereas bar2 is fine.

How can this be?

You'll also notice that bar3 is garbled. I'm not sure whether this is the same problem, or another problem.


Here is a simplified version of the interface and implementation of NRRanNormal:

class NRRanNormal {
public:

    NRRanNormal();

    ~NRRanNormal();

    NRRanNormal(const NRRanNormal& nrran);

    NRRanNormal& operator= (const NRRanNormal& nrran);

    double getMean()  const;    
    double getStdev() const;    
    long   getSeed()  const;

private:    
    double m_mean, m_stdev;     
    long m_seed;    
    Normaldev* stream; // underlying C struct RN generator

};

NRRanNormal::NRRanNormal() { // by default, N(0,1)
    m_mean  = 0.0;
    m_stdev = 1.0;
    m_seed  = 12345L;
    stream  = new Normaldev(m_mean, m_stdev, m_seed);
}

NRRanNormal::~NRRanNormal() { delete stream; }

NRRanNormal::NRRanNormal(const NRRanNormal& nrran) {
    stream = new Normaldev(nrran.getMean(),nrran.getStdev(),nrran.getSeed());
    *stream = *(nrran.stream);
}

NRRanNormal& NRRanNormal::operator= (const NRRanNormal& nrran) {            
    if(this == &nrran)
        return *this;

    delete stream;
    stream = new Normaldev(nrran.getMean(),nrran.getStdev(),nrran.getSeed());
    *stream = *(nrran.stream);

    return *this;
}

double NRRanNormal::getMean()  const { return m_mean; }    
double NRRanNormal::getStdev() const { return m_stdev; }    
long   NRRanNormal::getSeed()  const { return m_seed; }

The Normaldev struct is from Numerical Recipes 3d Edition.

Is there something wrong with my copy-assignment operator or copy constructor?


Here is Normaldev, stripped of the proprietary calculations.

typedef double Doub;        
typedef unsigned long long int Ullong;
typedef unsigned int Uint;

struct Ranq1 {
    Ullong v;
    Ranq1(Ullong j) : v(/* some long number here */) {
        /* proprietary calculations here */
    }
    inline Ullong int64() {
        /* proprietary calculations here */
    }
    inline Doub doub() { /* proprietary calculations here */ }
    inline Uint int32() { return (Uint)int64(); }
};

struct Normaldev : Ranq1 {
    Doub mu,sig;
    Normaldev(Doub mmu, Doub ssig, Ullong i):
    Ranq1(i), mu(mmu), sig(ssig){}
    Doub dev() {
        /* proprietary calculations here */
    }
};
Was it helpful?

Solution

This is your problem

NRRanNormal::NRRanNormal(const NRRanNormal& nrran) {
    stream = new Normaldev(nrran.getMean(),nrran.getStdev(),nrran.getSeed());
    *stream = *(nrran.stream);
}

should be

NRRanNormal::NRRanNormal(const NRRanNormal& nrran) : 
    m_mean(nrran.m_mean), 
    m_stdev(nrran.m_stdev), 
    m_seed(nrran.m_seed)
{
    stream = new Normaldev(nrran.getMean(),nrran.getStdev(),nrran.getSeed());
    *stream = *(nrran.stream);
}

Your copy constructor fails to copy the mean, stddev and seed. Your assignment operator has the same problem it should be

NRRanNormal& NRRanNormal::operator= (const NRRanNormal& nrran) {            
    if(this == &nrran)
        return *this;

    m_mean  = nrran.m_mean;
    m_stdev = nrran.m_stdev;
    m_seed  = nrran.m_seed;
    delete stream;
    stream = new Normaldev(nrran.getMean(),nrran.getStdev(),nrran.getSeed());
    *stream = *(nrran.stream);

    return *this;
}

I guess you were focussed so much on the tricky pointer in your class you forgot about the basic stuff.

BTW the code

MyClass A;
MyClass B = A;

is actually converted by the compiler to

MyClass A;
MyClass B(A);

in other words MyClass B = A; invokes the copy constructor (assuming A and B are the same type).

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top