Grobeu
Grobeu

Reputation: 29

C++ pointer inside a class returning weird values

So I'm making a simple class with two private pointers, two setters, two getters, a constructor and a destructor in the IDE CodeBlocks 13.12. With normal variables, the getters return the exact values, but using the pointers one return a weird value and the other return the exact value.

  1. Here is the header of my class :

    #ifndef COMPLEXE_H_INCLUDED
    #define COMPLEXE_H_INCLUDED
    
    #include <iostream>
    
    class Complexe{
    public:
        Complexe(const double re = 0.0, const double im = 0.0);
        ~Complexe();
    
        double getReel() const;
        double getImag() const;
    
        void setReel(double);
        void setImag(double);
    
    private:
        double* re;
        double* im;
    
    };
    
    #endif // COMPLEXE_H_INCLUDED
    
  2. Here is the source of my class

    #include "complexe.h"
    
    Complexe::Complexe(const double re, const double im)
    {
        double a = re;
        double b = im;
    
        this->re = &a;
        this->im = &b;
    }
    
    Complexe::~Complexe()
    {
        delete re;
        delete im;
    }
    
    double Complexe::getReel() const
    {
        return *re;
    }
    
    double Complexe::getImag() const
    {
        return *im;
    }
    
    void Complexe::setReel(double a)
    {
        *this->re = a; 
    }
    
    void Complexe::setImag(double a)
    {
        *this->im = a; 
    }
    
  3. Here is my main function

    int main()
    {
        Complexe cpl1(12, 3);
        cout << "C1 | " << "Re : " << cpl1.getReel()
           << ", Im : " << cpl1.getImag()
           << endl;
    
        cpl1.setReel(50);
        cpl1.setImag(50);
    
        cout << "C1 | " << "Re : " << cpl1.getReel()
            << ", Im : " << cpl1.getImag()
            << endl;
    
        Complexe * cpl2 = new Complexe(20,20);
        cout << "C2 | " << "Re : " << cpl2->getReel()
            << ", Im : " << cpl2->getImag()
            << endl;
    
        return 0;
    }
    
  4. Finally, here are the values that I get :

Result

What's wrong with my code and understanding of pointers ? Thanks

PS : Curiously, when I change my constructor code to :

Complexe::Complexe(const double re, const double im)
{
    double a = re;
    double b = im;

    this->re = &a;
    this->im = &a; // &a here too // or &b in both
}

I get the exact values :

Result with the same reference

Upvotes: 1

Views: 729

Answers (4)

Kaitain
Kaitain

Reputation: 960

It is a bad idea to have local variables that have the same names as class members, as you do in the case of re and im in your constructor.

Most experienced C++ practitioners give a prefix or suffix to all their class members. The most common is an m_ prefix. So in your case I would strongly advise calling your member variables

double* m_re;
double* m_im;

That way you could have a far less confusing ctor, as follows:

Complexe::Complexe(const double re, const double im)
{
    m_re = new double( re );
    m_im = new double( im );
}

I don't really understand why you want to store your doubles as heap objects in the first place, but that's your business.

Are you perhaps transitioning to C++ from Java or something like that? None of this code has the feel of idiomatic C++.

Upvotes: 0

cadaniluk
cadaniluk

Reputation: 15229

  1. a and b in the constructor are automatic variables and hence destroyed when the constructor returns.
    Any further access to re or im will thus lead to undefined behavior.

  2. Neither a nor b have been allocated on the free store, i.e., using new, so delete does not make sense and is undefined behavior. Just leave the destructor empty instead; the object will be destroyed automatically without leaking.

That "curiosity" you're experiencing is just a coincidence; undefined behavior is undefined, so it may very well work.


Notes:

  • As @LightnessRacesinOrbit stated in the comments to your question, you use pointers where they are unneeded. Just copying the arguments into the member functions suffices and will definitely make the code look cleaner. Of course, it works, but such use of pointers does not affect performance well and is just... yes, pointless.

Upvotes: 7

Xirema
Xirema

Reputation: 20386

Your understanding of pointers isn't what's wrong, per-se. The problem is that you don't understand what happens here:

Complexe::Complexe(const double re, const double im)
{
    double a = re;
    double b = im;

    this->re = &a;
    this->im = &b;
}

What's happening in this code is that doubles a and b are being allocated on the stack. re and im are being set to point to the addresses of a and b respectively. Then, when the constructor returns, doubles a and b are lost as the stack returns back to the calling function. As a result, re and im are pointing to invalid locations on the stack.

If you absolutely need re and im to be pointers, then you need to rewrite it like so:

Complexe::Complexe(const double re, const double im)
{
    this->re = new double(re);
    this->im = new double(im);
}

And then you'll need to make sure you include in the Destructor:

Complexe::~Complexe()
{
    delete re;
    delete im;
}

Upvotes: 5

ForeverStudent
ForeverStudent

Reputation: 2537

inside your constructor you have:

Complexe::Complexe(const double re, const double im)
{
    double a = re;  //a is a local variable created in this stack frame
                    //content of re is copied inside a
    double b = im;

    this->re = &a;  //you are taking the address of a variable that is created on
                    // the stack and assigning it to a pointer that will persist
    this->im = &b;
} 
//when the constructor is finishes executing, a and b are destroyed,
//but this->re and this->im will continue to hold their address. 

This induces undefined behaviour. As for why such code my sometimes work: undefined behaviour does not always mean a run time error. It simply means that the language specification has not defined a behaviour for the situation at hand and as such, the compiler has the liberty to have any implementation in place. You could get the value you want, you could get garbage.

Upvotes: 0

Related Questions