I have this piece 开发者_如何学Goof code and for some reason it creates 6 memory leaks and I can't figure out where.
Lemon::Lemon()
{
this->nrOf=0;
int x=10;
this->matrix=new int*[x];
this->lines=new string[x];
for (int i = 0; i < x; ++i)
this->matrix[i] = new int[x];
for(int i=0;i<x;++i)
for(int j=0;j<x;++j)
this->matrix[i][j]=-1;
}
Lemon::Lemon(int n)
{
this->x=this->nrOf;
this->matrix=new int*[x];
this->lines=new string[x];
for (int i = 0; i < x; ++i)
this->matrix[i] = new int[x];
for(int i=0;i<x;++i)
for(int j=0;j<x;++j)
this->matrix[i][j]=-1;
}
Lemon::~Lemon()
{
for(int i=0;i<this->nrOf;i++)
{
delete []this->matrix[i];
}
delete []this->matrix;
delete []this->lines;
}
Any kind of help is appreciated.
At this point:
this->x=this->nrOf;
nrOf has not been initialised - you have undefined behavior.
And possibly you would benefit from using std::vector.
The default constructor initializes nrOf with 0, but you allocate a matrix of 10x10. Which means the destructor will not run the for loop, since nrOf is 0.
BTW, you don't have to prefix everything with this. It kinda makes the code harder to read. You should use this only to eliminate ambiguity.
You don't set nrOf
to the number of allocated matrix rows in either constructor.
Try this as your only constructor
Lemon::Lemon(int n = 10) // default constructor
{
nrOf = n;
matrix = new int*[nrOf];
lines = new string[nrOf];
for (int i = 0; i < nrOf; ++i)
matrix[i] = new int[nrOf];
for(int i=0; i<nrOf; ++i)
for(int j=0; j<nrOf; ++j)
this->matrix[i][j] = -1;
}
You seem to have both a member variable int x
(used in Lemon::Lemon(int n)
) as well as a local variable int x
in Lemon::Lemon()
. Also, you do not consistently set nrOf
in your constructors.
Note 1:
Every new-expression in your code is possible memory leak because your code is awfully exception unsafe: if any of your new
expressions throw an exception all your already new-allocated storage is leaked.
Note 2:
Your first constructor does not semantically match your destructor (even if we assume that everything goes fine without throwing): constructor assigns zero value to this->nrOf
but allocates 10 elements of this->matrix
array and assign allocated array to each element BUT destructor expects this->nrOf
elements of this->matrix
(arrays) to delete so 0 elements is deleted and thus 10 elements are leaked.
Side note: your this->x
(that exists according to second constructor) is not initialized by first constructor.
Note 3:
Your second constructor is simply corrupt: you never initialize this->nrOf
variable but you use it like it contains something valid.
ADD:
Good advise here would be: never use arrays and new
/new[]
/delete
/delete[]
expressions. Use std::vector
in your situation instead. This way you may never care about memory leaks, exception safety, etc.
Your matrix
would look like this:
std::vector< std::vector<int> > matrix;
....................
....................
n = 10;
m = 20;
matrix.resize(n);
for( size_t j = 0; j < n; ++j )
{
matrix[j].resize(m);
for( size_t k = 0; k < m; ++k )
{
matrix[j][k] = j+k;
}
}
精彩评论