Re: [eigen] API change proposal

[Tim Hutt <tdhutt@xxxxxxxxx>]

2009/7/24 Markus Benjamin Fröb <grey_earl@xxxxxx>:
> Matrix<>* LU::solve(const Matrix<>& b) {
>  return the solution if it exists, else null
> }

That is a *terrible* API - the sort I'd expect of a C library. Not
only does it force manual memory management, but functions returning
pointers are a very bad idea because it is never documented who should
delete the Matrix<> which leads to memory leaks. Consider all the
mistakes this makes possible:

class Matrix1
{
publc:
   Matrix1* solve()
   {
      cachedSolution = ...;
      return &cachedSolution;
   }
private:
   Matrix1 cachedSolution;
}

class Matrix2
{
public:
   Matrix1* solve()
   {
      Matrix2* solution = new Matrix2;
      *solution = ...;
      return solution;
   }
}

void mistakes(const Matrix1& m1, const Matrix2& m2)
{
  Matrix1* lu = m2.solve();
 ...
  if (foo)
    return; // lu not freed.

  Matrix2* lu2 = m1.solve();


  if (foo)
  {
    delete lu2; // shouldn't have deleted this.
    return;
  }

} // lu still not freed.