Rebulid a variable that sent by reference

Question

this is my code

void SMatrix::pow(int power, SMatrix & result)
{
        if (this->rowSize != this->colSize || this->rowSize != result.rowSize || this->colSize != result.colSize || power <= 0)
        {
            delete & result;
            result = new SMatrix (result.rowSize, result.colSize);
        }
}

Im trying to delete this result in this case, and send it as new SMatrix. How can i do it? (reuslt = newM..... ask me for SMatrix *, but it doesn't work with &).

In the main i can build it like that: SMatrix * s = new SMatrix(4, 4); or SMatrix s(4, 4); (pointer or not).

What happens if `result` is on the stack? Please have a rethink — Ed Heal, Dec 28 '15 at 08:22
Why do you `delete &result`? Are you sure the caller will pass a dynamically allocated object? If they don't, bamm! Just use value semantics. `result = SMatrix (result.rowSize, result.colSize);` or `result.swap(SMatrix(result.rowSize, result.colSize));` if you have an efficient swap. And get rid of the `delete`. — juanchopanza, Dec 28 '15 at 08:22
It would be legal to write `result.~SMatrix(); new(&result) SMatrix(result.rowSize, result.colSize);` . But this is more error-prone and probably less efficient than using assignment as in Mats Petersson's solution — M.M, Dec 28 '15 at 08:44

score 4 · Accepted Answer · edited Dec 28 '15 at 08:41

This code is just "doing it wrong".

If you have a reference argument, then the implied effect is that the ownership of any pointer to it belongs to the caller.

void SMatrix::pow(int power, SMatrix & result)
{
        if (this->rowSize != this->colSize || this->rowSize != result.rowSize || this->colSize != result.colSize || power <= 0)
        {
            delete & result;
            result = new SMatrix (result.rowSize, result.colSize);
        }
}

If your SMatrix doesn't have a proper operator=, then you should have one. In other words, the correct thing should happen if you do:

        if (rowSize != colSize || rowSize != result.rowSize || colSize != result.colSize || power <= 0)
        {
            result = SMatrix (result.rowSize, result.colSize);
        }

(Note that I removed both the delete line and the new operator)

If this, for some reason, won't work correctly, then you need to fix that, not rely on how the orignal data was allocated.

Yes, it's unnecessary, but I generally don't edit the OP's code more than necessary - it just makes it harder to spot the "actually needed" changes, vs. "cosmetic/non-functional" changes. — Mats Petersson, Dec 28 '15 at 08:42

score 2 · Answer 2 · answered Dec 28 '15 at 08:26

2

        delete & result;
        result = new SMatrix (result.rowSize, result.colSize);

You can't delete an object and then call operator= on it. You're doing the equivalent of this:

std::string* j = new std::string ("hello");
delete j;
*j = "goodbye"; // Oops, there's no string whose value we can set

I think you probably just want result = SMatrix (result.rowSize, result.colSize);. You want to change the value of result, you don't want to delete or create anything dynamic.

answered Dec 28 '15 at 08:26

David Schwartz

179,497
17
214
278

For a start in the posters code `result` is not a pointer - so there is a mismatch with `new`. Also the poster is using `result.xxxx` in the construction – Ed Heal Dec 28 '15 at 08:33
@EdHeal I know his code doesn't use a pointer. I switched it to a pointer (keeping everything else the same) to make things easier to understand. I don't know what you mean by "mismatch with new". – David Schwartz Dec 28 '15 at 08:35
@EdHeal Right, so? If you're talking about what he has on the right side of the `=` sign, it doesn't matter. Calling `operator=` would be incorrect regardless of what you're trying to set it equal to. – David Schwartz Dec 28 '15 at 08:37
LHS is a different type to the RHS – Ed Heal Dec 28 '15 at 08:38
@EdHeal Fortunately in C++ one can write converting constructors and assignment operators. But I suspect this is actually the problem OP is running into, i.e. they are yet to run into a runtime memory problem. – juanchopanza Dec 28 '15 at 08:39
1

@EdHeal Nothing wrong with that, since the thing on the left is a class. That might also be an error in his code, but it's much more important that he understand that he's invoking `operator=` on an object that no longer exists. – David Schwartz Dec 28 '15 at 08:39

Rebulid a variable that sent by reference

2 Answers2