class Vector2d
{
private:
double m_x, m_y;
public:
Vector2d(double x, double y) { m_x = x; m_y = y; }
virtual ~Vector2d() {}
double getX() { return m_x; }
double getY() { return m_y; }
const Vector2d& operator+ (const Vector2d& other)
{
// How do I do this without a memory leak?
//
// I get compiler warnings on this:
return Vector2d(m_x + other.m_x, m_y + other.m_y);
// The compiler warnings are obviously because I''m
// returning a reference to a stack allocated
// structure, which will get blown away on the next
// function call.
//
// The only alternative I can think of is this:
return *new Vector2d(m_x + other.m_x, m_y + other.m_y);
// While the above compiles without warnings, it seems
// like a memory leak. Does the compiler handle this
// somehow, or is this a genuine memory leak?
//
// If it is a memory leak, what''s the correct solution
// to this problem?
}
// other operators
} [C++] operator overloading and memory management.
Author
Consider a class with overloaded arithmetic operators, following the immutable pattern. I seem to have embedded my question in the code:
The answer is simple, don''t return a reference, returning a
reference when creating a new object never works, instead use
this form:
class Vector2D
{
... etc ...
Vector2D operator+ (const Vector2D &v) {
return Vector2D(m_x + v.m_x, m_y + v.m_y);
}
};
reference when creating a new object never works, instead use
this form:
class Vector2D
{
... etc ...
Vector2D operator+ (const Vector2D &v) {
return Vector2D(m_x + v.m_x, m_y + v.m_y);
}
};
I believe the correct way looks like this.
const Vector2d& operator+ (const Vector2d& other)
{
static Vector2d retv(0,0);
retv = Vector2d(m_x + other.m_x, m_y + other.m_y);
return retv;
}
const Vector2d& operator+ (const Vector2d& other)
{
static Vector2d retv(0,0);
retv = Vector2d(m_x + other.m_x, m_y + other.m_y);
return retv;
}
quote:Original post by Mark_C
I believe the correct way looks like this.
const Vector2d& operator+ (const Vector2d& other)
{
static Vector2d retv(0,0);
retv = Vector2d(m_x + other.m_x, m_y + other.m_y);
return retv;
}
If you do that, you have this problem :
Vector2d a,b,c,d;
...
(a+b) == (c+d) will always be true, since both (a+b) and (c+d) 'refer' to the same static variable 'retv'. So, say, (a+b) first sets the value of retv, and returns a reference to retv, then (c+d) changes the value of retv, and returns a reference to retv... obviously retv == retv.
Edited by - Fruny on December 13, 2001 2:18:33 PM
quote:Original post by
While the above compiles without warnings, it seems like a memory leak. Does the compiler handle this somehow, or is this a genuine memory leak?
Technically, no. If you call delete on the address of the const reference to Vector2d, then everything will be fine. However, it does not make very much sense, and writing an arithmetic operator that creates objects on the free store is asking for trouble.
Now I have a couple of points to make regarding the efficiency of your class.
First, if you are not deriving classes from Vector2d that require destructors called by classes using them as a base class for other classes which themselves require destructors, and calling delete on base class pointers within the hierarchy, then the virtual destructor is unnecessary.
Why mention this? Because if it is a vector class, then chances are that you will be creating a lot of them. Classes that have virtual member functions have an added per-instance cost, which is usually the size of a single pointer. If the virtual destructor is the only virtual member function and is unnecessary, then removing it will give you a smaller vector class. (FYI, the size of a pointer is four bytes on Win32.)
(And if you do not need a virtual destructor, then defining an empty destructor is equally unnecessary and may prevent the compiler from performing other optimizations, especially given the return-by-value nature of your arithmetic operators, which implies destructor calls.)
Second, when you assign values to member variables within a constructor, the preferred syntax is:
Vector2d(double x, double y) : m_x(x), m_y(y) {}Although your method will work as well. The exception to this rule is to not directly allocate objects on the free store until you get inside the blocks, otherwise you can cause memory leaks if exceptions occur.
Third, if you do not need the constructor call then I would eliminate that in favor of speed. If you allocate an array of one thousand vectors than that will be one thousand constructor calls, followed by the initialization that you will need to do anyway. (I do not know of anyone who needs an array of one thousand vectors with the same values: 0,0.). You can eliminate the requirement of calling a constructor by adding an empty default constructor:
Vector2d() {}- null_pointer
This is why, by the way, vectors and matrices are a bad subject for classes--the typical first try yields huge amounts of temporaries and copies. To do this "the right way", you should have vector''s operators return proxies of their operations, and the assignment operator evaluate the proxies. This gets extremely complicated.
Author
Hmm... Too many replies that are FAR off topic. I was just using this code as an example. The question was "How do I manage that memory correctly?"
Are you telling me its simply impossible to use the immutable pattern correctly with overloaded arithmetic operators in C++?
Are you telling me its simply impossible to use the immutable pattern correctly with overloaded arithmetic operators in C++?
No, we''re saying return by value, not by reference.
Read "Effective C++" by Meyers. He goes through how to write a class (a string class for his example) with correct operator overloads so that it works just like a built-in type.
Read "Effective C++" by Meyers. He goes through how to write a class (a string class for his example) with correct operator overloads so that it works just like a built-in type.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
