Reputation: 29
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.
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
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;
}
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;
}
Finally, here are the values that I get :
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
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
Reputation: 15229
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.
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:
Upvotes: 7
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
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