c++ return value when try and catch

Question

hi what should I do when WARANING "control reaches end of non-void function" happens ? my overloaded operator has try and catch and returns *this ; in the try scope.

I'm using Eclipse , G++ is the compiler, UBUNTU linux

NNmatrix & operator*=(const NNmatrix<T> &mtrxB)
        {
            // A=2*3 B=3*4  return C=2*4
            const UINT aRows = this->size1();
            const UINT aCols = this->size2();
            const UINT bRows = mtrxB.size1();
            const UINT bCols = mtrxB.size2();
            try
            {
                // if cols of first(this) matrix == rows of second matrix
                if (aCols != bRows) throw bad_alloc();
                const UINT cRows = aRows;// = rows of first matrix
                const UINT cCols = bCols; // = cols of second matrix
                NNmatrix mtrxC(cRows, cCols);
                T val;
                for (UINT i = 0; i < cRows; i++)
                {
                    for (UINT j = 0; j < cCols; j++)
                    {
                        val = 0;
                        for (UINT k = 0; k < bRows; k++)
                        {
                            val += this->matrix.at(i).at(k) * mtrxB.getElement(k, j);
                        }
                        mtrxC.setElement(i, j, val);
                    }
                }
                *this = mtrxC;
                mtrxC.clear();
                return *this;
            }
            catch (exception& e)
            {
                cout<<"Dimension don't match: ("<<aRows<<","<<aCols<<") ("<<bRows<<","<<bCols<<")"<<endl;
            }
        }

Answer 1

I don't know your larger design, but as a general rule overloaded operators should never have run-time failures at all (e.g. "=", "*", etc. should never throw exceptions). This is because users expect them to act like the +,-,etc. for numbers.

Think about how a user is going to call this. It looks like you want them to have the convenience of something like:

NNMatrix<int> matrixA; // Obviously with real assignments...
NNMatrix<int> matrixB;
matrixA *= matrixB;

Now, if you really need to support matrices whose sizes are set at run-time, you will have Add, Subtract, and Multiply operations that can fail. I personally would not overload the actual operators ,+,-,=, etc. on such an object because you can only signal an error with an exception.

Think about it. You're requiring the user to have something like this to be safe on EVERY call to your matrix operators:

try {
   matrixA *= matrixB;
}
catch(bad_alloc& ba) {
   // Handle runtime error
}

Now, what are the odds that somebody is going to wrap every one of your calls in a try-catch? Even if they do, it is no cleaner than the alternative, which is using an ordinary member function like:

bool NNMatrix<T>::MultiplyBy(const NNMatrix<T>& other);

If you return false and don't do the multiply when the dimensions are mismatched, you get the same behavior as before. Now the caller only needs something like:

if(!matrixA.MultiplyBy(matrixB)) {
   // Handle runtime error
}

This is better in that this will never cause a crash if they forget to check the return. It is obvious from the public API that users need to check for an error, and that the operation either totally succeeds or fails. It admittedly still isn't pretty and the user won't get what he expects if he doesn't have the 'if()' logic, but at least he has been warned. As far as I know this is about the best you can do if you really have to support matrices with an unknown # of rows and columns at compile time.

Answer 2

Basically I get it that your operator looks like:

Object& operator=(Object const& rhs)
{
  try
  {
    // something
    return *this;
  }
  catch(std::exception& e)
  {
    std::cerr << e.what() << '\n';
  }
}

Now the question is, which value do you return when an exception is thrown (and caught) ? The response is, you do not return anything, so what is the compiler supposed to do ?

You need to decide what to do on the catch path:

• either throw something (perhaps the same exception, or another of your liking)
• or return *this if you could live with the error

But in the case of an assignment operator, I would strongly encourage you NOT to have any exception thrown in, which is subtly different than blindly applying a try / catch block.

EDIT:

A much simpler approach to your issue would be to check first, throw your exception (without modifying anything) if it doesn't match, and then go about your code without any worry about an exception occurring.

It's a general way of developing in exception-land: do what may throw first, and then you won't have to worry about exceptions popping here and there.

Also, as a remark, don't you dare throwing bad_alloc! You cannot blindly pick an existing exception and use it for your own convenience: an exception type carry meaning and bad_alloc means that the system could not fulfill your request for memory, not that some matrix implementation has gone awry.

Answer 3

You need to make sure all code paths return a value if the function returns anything but void.

If you are handling exceptions internally to this function without rethrowing, why not just return *this; at the end of the function unconditionally, instead of from inside the try block?

EDIT: per @Mark's comment below, simply moving the return statement hides an error that is fatal in the context of the requested operation, and makes the library rather unreliable in the process. Better to propagate the exception, if that's how you are going to handle in-place multiplication errors (which seems a reasonable approach).

Answer 4

The solution to your problem is this :

NNmatrix & operator*=(const NNmatrix<T> &mtrxB)
        {
            // A=2*3 B=3*4  return C=2*4
            const UINT aRows = this->size1();
            const UINT aCols = this->size2();
            const UINT bRows = mtrxB.size1();
            const UINT bCols = mtrxB.size2();
            try
            {
                // if cols of first(this) matrix == rows of second matrix
                if (aCols != bRows) throw bad_alloc();
                const UINT cRows = aRows;// = rows of first matrix
                const UINT cCols = bCols; // = cols of second matrix
                NNmatrix mtrxC(cRows, cCols);
                T val;
                for (UINT i = 0; i < cRows; i++)
                {
                    for (UINT j = 0; j < cCols; j++)
                    {
                        val = 0;
                        for (UINT k = 0; k < bRows; k++)
                        {
                            val += this->matrix.at(i).at(k) * mtrxB.getElement(k, j);
                        }
                        mtrxC.setElement(i, j, val);
                    }
                }
                *this = mtrxC;
                mtrxC.clear();
            }
            catch (exception& e)
            {
                cout<<"Dimension don't match: ("<<aRows<<","<<aCols<<") ("<<bRows<<","<<bCols<<")"<<endl;
                // let the exception propagate
                throw;
            }

            // always return *this
            return *this;
        }

Answer 5

The warning refers to the fact that there is a non-exceptional control path out of the function that does not have a return statement.

Does your catch scope have a similar return? If it is not supposed to return, you might want to rethrow.