CODEWITHSUNDEEP
c++referenceconstructor
user2406944
user2406944

Reputation:

Error: No instance of constructor matches the argument list

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

Upvotes: 6

Views: 73296

Answers (3)

Oswald
Oswald

Reputation: 31647

  • (*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.

Upvotes: 1

Dietmar Kühl
Dietmar Kühl

Reputation: 153810

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!

Upvotes: 1

Daniel Frey
Daniel Frey

Reputation: 56863

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.

Upvotes: 13

Related Questions