Error: No instance of constructor matches the argument list

Question

As part of an assignment we have been asked to create a Vector3D class which uses memory allocated on the Heap. I have a Vector3DHeap class with the following constructor.

Vector3DHeap::Vector3DHeap(float& x, float& y, float& z)
{
    this->x = &x;
    this->y = &y;
    this->z = &z;
}

If I want to get a unit vector, I was expecting the be able to do the following. This gives the error message "No instance of constructor matches the argument list, argument types are (float, float, float).

Vector3DHeap* Vector3DHeap::getUnitVector()
{
    float m = *getMagnitude();

    return new Vector3DHeap((*x / m), (*y / m), (*z / m));
}

The compiler is happy if I define three float variables, a, b and c and pass these to the constructor. What is wrong with the code above?

Vector3DHeap* Vector3DHeap::getUnitVector()
{
    float m = *getMagnitude();

    float a, b, c;

    a = *x / m;
    b = *y / m;
    c = *z / m;

    return new Vector3DHeap(a, b, c);
}

Many thanks, George

Solution 1

Your problem with the first version is that your compiler is trying to prevent a bug.

Your problem with the second version is that you outsmarted your compiler and successfully managed to create a bug.

Given your constructor, you want to store pointers to the float values that are passed by reference. Since your second version now calls the constructor with references to the local variables float a, b, c;, you created an instance of Vector3DHeap which references them. But as soon as getUnitVector returns, those variables no longer exist and the references stored in Vector3DHeap became dangling references.

The solution is not to store pointers inside Vector3DHeap or to create copies of the parameters:

Vector3DHeap::Vector3DHeap(float x, float y, float z)
{
    this->x = new float(x);
    this->y = new float(y);
    this->z = new float(z);
}

Make sure that you properly delete the stored floats, though.

Solution 2

It is good that the compiler stopped you from binding a reference to a temporary because otherwise you would have ended up with an object pointing to already destroyed objects: The expressions *x / m and similar each yield a temporary float object which will disappear at the end of the expression. Trying to bind a temporary to a non-const reference will fail.

However, I doubt that you really want to do any of that: you shouldn't use pointers unless you really know that you need to use pointers! Your constructor should probably rather look like this:

Vector3DHeap::Vector3DHeap(float x, float y, float z)
    : x(x), y(y), z(z) {
}

where the members are, of course, also of type float. getMagnitude() should return a float, too. ... as should getUnitVector() return a Vector3DHeap rather than a pointer to it!

Solution 3

• (*x / m) is a temporary object.
• Vector3DHeap(float& x, float& y, float& z) requires a non-const reference as first parameter.

You can't pass a temporary object to a function that expects a non-const reference. See https://stackoverflow.com/questions/13826897#13827042 for details why C++ does not want to allow this.

Comments

[Admin]

Thanks for your reply. We have been asked to "rewrite the class (call it Vector3DHeap) so that variables are stored on the heap rather than the stack." If return a float and a Vector3DHeap rather than a pointer, will those not be on the Stack instead of the Heap?

[David G]

@GeorgeRobinson Objects are stored on the heap whenever you use new or new[] to initialize them.

[Admin]

I must return a pointer type if I have return new Vector3DHeap(x, y, z) since new returns a pointer? I am confused since you said "getUnitVector() return a Vector3DHeap rather than a pointer to it!"

[Dietmar Kühl]

@GeorgeRobinson: Mandating to store individual floats on the heap is extremely stupid and I certainly hope your teacher did not mean that! Even if the objects need to be on the heap you'd allocate (and realese) them by a suitable class: returning pointer for objects not maintained otherwise generally dies not work.