STL Memory leak?

[schliz]

As I am saving pointer of CScreen in the list. Do I have to delete the
CScreens objects manually?

Example 1:

CScreen* newScreen = new CScreen();
list<CScreen*> m_screenList;
m_screenList.push_back(newScreen);
m_screenList.clear(); --> does it create a memory leak?

Example 2:

CScreen newScreen;
list<CScreen> m_screenList;
m_screenList.push_back(newScreen);
m_screenList.clear(); //In this case it is destroyed correctly. Is
this correct?


Thank you for your help.
Georg

 

[Vaclav Haisman]

schliz wrote, On 7.4.2009 9:36:

As I am saving pointer of CScreen in the list. Do I have to delete the
CScreens objects manually?

Example 1:

CScreen* newScreen = new CScreen();
list<CScreen*> m_screenList;
m_screenList.push_back(newScreen);
m_screenList.clear(); --> does it create a memory leak? Yes.


Example 2:

CScreen newScreen;
list<CScreen> m_screenList;
m_screenList.push_back(newScreen);
m_screenList.clear(); //In this case it is destroyed correctly. Is
this correct? Yes.



Thank you for your help.
Georg

C++ is no Java. It does not have GC.

 

[Tonni Tielens]

As I am saving pointer of CScreen in the list. Do I have to delete the
CScreens objects manually?

Yes. So yes: example 1 creates a memory leak and yes: example 2 is
correct.

If you really want to store pointers in your list, you can manually
free the memory by iterating over the list, use the DeleteObject
function object described in Scott Meijers' Effective STL or use
boost's ptr_list or shared_ptr.

 

[Daniel Pitts]

By the way, your naming convention is very verbose, and thus obscures
things. The scope of a variable should play a part in the length of its
name; smaller scope, shorter name. But this is just a style issue.

Not to start a flame war, but I disagree. Names should *always* reflect
the semantics of what they represent as concisely as possible. Take
away all you can, but no more.

For example:
if (useNode) {
Node n; // n is to short, even for this short scope.
n.doStuff();
}

Yes, you can see n and very quickly look up to see it is a Node, but you
save a couple of brain-cycles if you use the following.

if (useNode) {
Node node;
node.doStuff();
}

It doesn't obscure anything. Of course, these things are always value
judgments, and there is often reason to stray.

For example, I've seen the following two functions, which one is easier
to understand:

char *strcpy(char*a, char*b)
{
char *c = a;
while ((*b = *a) != 0)
{
++b;
++a;
}
return c;
}

char *strcpy(char *source, char *dest)
{
char *ret = source;
while ((*dest = *source) != 0)
{
++dest;
++source;
}
return ret;
}

Exact same function, different names, the second one is way easier to
understand IMO.

 

[Victor Bazarov]

Not to start a flame war, but I disagree. Names should *always* reflect
the semantics of what they represent as concisely as possible. Take
away all you can, but no more.

For example:
if (useNode) {
Node n; // n is to short, even for this short scope.
n.doStuff();
}

Yes, you can see n and very quickly look up to see it is a Node, but you
save a couple of brain-cycles if you use the following.

if (useNode) {
Node node;
node.doStuff();
}

It doesn't obscure anything. Of course, these things are always value
judgments, and there is often reason to stray.

For example, I've seen the following two functions, which one is easier
to understand:

char *strcpy(char*a, char*b)

... char const* b)

{
char *c = a;
while ((*b = *a) != 0)
{
++b;
++a;
}
return c;
}

char *strcpy(char *source, char *dest)
{
char *ret = source;
while ((*dest = *source) != 0)
{
++dest;
++source;
}
return ret;
}

Exact same function, different names, the second one is way easier to
understand IMO.

So, if I understand you correctly, the form of 'strcpy' like this

char *strcpy(char* a, char const* b)
{
char *c = a;
while (*a++ = *b++)
;
return c;
}

is no good, right? And mostly because of the single-letter names, correct?

I think I get your point. However, functions like those are often
write-only. Nobody cares about the body of 'strcpy'. It works, and
that's about all we need to know. While it's being debugged, there is
no need to name the arguments in a particular way, since the programmer
knows what the meaning of the arguments/variables is. Once debugged, it
should be put in the vault, documented, and never looked at again.

V

 

[James Kanze]

Not to start a flame war, but I disagree. Names should
*always* reflect the semantics of what they represent as
concisely as possible. Take away all you can, but no more.

There's no arguing with that, but the original poster had a lot
of misinformation in his names. CScreen, rather than Screen
(which I would generally interpret as indicating that the class
was part of MFC) and the m_ prefixes on the local variables
(which is a common convention for member variables).

For example:
if (useNode) {
Node n; // n is to short, even for this short scope.
n.doStuff();
}

Yes, you can see n and very quickly look up to see it is a
Node, but you save a couple of brain-cycles if you use the
following.

if (useNode) {
Node node;
node.doStuff();
}

I'd probably write it like you just did, but as long as the
scope is only two or three lines, I could live (just barely)
with the first.

On the other hand,

for ( int i = 0 ; i < count ; ++ i ) {
a[ i ] *= 2 ;
}

is actually clearer than:

for ( int indexIntoArray = 0 ; indexIntoArray < count ; ++
indexIntoArray ) {
a[ indexIntoArray ] *= 2 ;
}

There are some universal conventions, including that of using i,
j, k for local indexes, which should be respected.

It doesn't obscure anything. Of course, these things are
always value judgments, and there is often reason to stray.

For example, I've seen the following two functions, which one
is easier to understand:

char *strcpy(char*a, char*b)
{
char *c = a;
while ((*b = *a) != 0)
{
++b;
++a;
}
return c;
}

char *strcpy(char *source, char *dest)
{
char *ret = source;
while ((*dest = *source) != 0)
{
++dest;
++source;
}
return ret;
}

Exact same function, different names, the second one is way
easier to understand IMO.

Neither are really very good. An assignment embedded in a flow
control statement is an abomination. (But then, so are the
semantics and the interface of the given function to begin
with.)

And of course, I notice that you've adopted a convention for
abbreviating names as well: why not sourceString and
destinationString? (Objects should be qualified nouns, and
abbreviations aren't used, according to the usual naming rules.)

 

[Daniel Pitts]

There's no arguing with that, but the original poster had a lot
of misinformation in his names. CScreen, rather than Screen
(which I would generally interpret as indicating that the class
was part of MFC) and the m_ prefixes on the local variables
(which is a common convention for member variables).

Ah, I wasn't thinking about that part. I was actually assuming that the
OP was cutting from several places in his project, so the variable name
made sense for the original context.

if (useNode) {
Node node;
node.doStuff();
}

I'd probably write it like you just did, but as long as the
scope is only two or three lines, I could live (just barely)
with the first.

On the other hand,

for ( int i = 0 ; i < count ; ++ i ) {
a[ i ] *= 2 ;
}

is actually clearer than:

for ( int indexIntoArray = 0 ; indexIntoArray < count ; ++
indexIntoArray ) {
a[ indexIntoArray ] *= 2 ;
}

There are some universal conventions, including that of using i,
j, k for local indexes, which should be respected.

Absolutely, I agree with that, although "a" should be named something
better.

It doesn't obscure anything. Of course, these things are
always value judgments, and there is often reason to stray.

For example, I've seen the following two functions, which one
is easier to understand:

[snipping code]

Exact same function, different names, the second one is way
easier to understand IMO.

Neither are really very good. An assignment embedded in a flow
control statement is an abomination. (But then, so are the
semantics and the interface of the given function to begin
with.)

And of course, I notice that you've adopted a convention for
abbreviating names as well: why not sourceString and
destinationString? (Objects should be qualified nouns, and
abbreviations aren't used, according to the usual naming rules.)

Why should they be qualified? Are your referred to as PersonJames or
JamesPerson? or, pJames to adopt some M$ notation...

My point with that is you should choose the simplest name that conveys
enough information. The difficulty, as always, is that both "simplest"
and "enough information" are subjective terms. Hopefully the "average"
person will have an "average" interpretation of
simplest-with-enough-information.

 

[Victor Bazarov]

[..]
And of course, I notice that you've adopted a convention for
abbreviating names as well: why not sourceString and
destinationString? (Objects should be qualified nouns, and
abbreviations aren't used, according to the usual naming rules.)

Why should they be qualified? Are your referred to as PersonJames or
JamesPerson? or, pJames to adopt some M$ notation...

My point with that is you should choose the simplest name that conveys
enough information.

.... *given any particular context* (and I believe it's important). For
example, our development group has several people named James. In a
conversation we usually have to qualify which James we're talking about.
AFA 'strcmp' is concerned, it's probably not complex enough a context
to qualify 'source' and 'destination' with an additional type indicator...

> The difficulty, as always, is that both "simplest"
and "enough information" are subjective terms. Hopefully the "average"
person will have an "average" interpretation of
simplest-with-enough-information.

That's what the coding style documents are for. It is not inconceivable
to create a set of rules for variable naming. Once it's done, the
subjective element is out of the equation. Of course, the document has
to allow for growth, evolution, and not only in the tools' capabilities,
but in the team's members' skills to read and interpret code. I've seen
coding style docs with the requirement that a variable should always
have at least two letters in its name. I can't write

for (int i = 0; i < somecount; ++i)

and instead have to use 'ii':

for (int ii = 0; ii < somecount; ++ii)

Nobody remembered why it was established. So, common sense and periodic
reviews of the policies are important, too.

V

 

[red floyd]

Aha!  Maybe that's the origin of this hideous "ix" name I keep seeing.
I always have to remind myself that the value isn't 9.

I've never seen it codified, but I tend to use "j" instead of "i" for
a simple loop index.
My rationale? It's a heck of a lot easier to find (greater S/N ratio
when looking for "j").

 

[Arne Mertz]

for (int i = 0; i < somecount; ++i)

and instead have to use 'ii':

for (int ii = 0; ii < somecount; ++ii)

Nobody remembered why it was established. So, common sense and periodic
reviews of the policies are important, too.

V

Try to grep the first example for "i" - and then try to grep the
second one for "ii". Thats one of the often read reasons for that
kind of style guides.

In one of the books I have here, there is a cite of Grady Booch
saying "Clean Code reads like well-written prose" - and I think
that's what is important about names: they should make the code easy
to read without having to look at a particular variable's or
function's definition to understand it.

Greets
A

 

[Arne Mertz]

So, if I understand you correctly, the form of 'strcpy' like this

char *strcpy(char* a, char const* b)
{
char *c = a;
while (*a++ = *b++)
;
return c;
}

is no good, right? And mostly because of the single-letter names, correct?

I think I get your point. However, functions like those are often
write-only. Nobody cares about the body of 'strcpy'. It works, and
that's about all we need to know. While it's being debugged, there is
no need to name the arguments in a particular way, since the programmer
knows what the meaning of the arguments/variables is. Once debugged, it
should be put in the vault, documented, and never looked at again.

For the bodies of library functions, I agree. For the functions'
names and parameter declarations I do not. I'd rather have the
parameters named source and dest as in the other example, just to be
reminded which order I have to pass the arguments.
In development, names should be meaningful as well - until the code
is stable an it is proven that noone will have to go through it
reading otherwise meaningless names.

greets
A

 

[James Kanze]

[...]

Why should they be qualified? Are your referred to as
PersonJames or JamesPerson? or, pJames to adopt some M$
notation...

It's just an old rule I thought I'd throw out: names of types
are unqualified nouns, names of variables are qualified nouns,
and names of functions are verbs (or verbal phrases). As a
general rule for coding, it's not bad, and it does avoid the
need for a lot of special rules. Thus, for example a "setter"
for state will assign "newState" to "currentState", and you no
longer have the problem of function parameter name and member
name conflicting.

(As for your example, there's a very real sense that a propre
name works as a "qualified noun". James specifies a specific
person, and not just any person. And if there were two James,
one a person, and one a dog, you might say James the person, as
opposed to James the dog.)

In the context I cited it, however, I was being somewhat
facetious. It's a good general rule, but almost by definition,
general rules don't apply to all specific cases. (Otherwise,
it's an absolute rule.) In practice, I do use the identifiers
source and dest when the name of the function makes it clear
what "source" and "dest" refer to; for that matter, I'll even
use them in cases where the type (e.g. std::istream&) makes it
evident. And I use "dest", and not "destination", even though
it is an abbreviation, and the general rule also says no
abbreviations. (It is, in fact, almost the only abbreviation I
use. The only other one which comes to mind is ptr, for
pointer.)

As an absolute rule (which requires compelling justification to
violate), you should be consistent. If you do use "dest" in one
place, rather than "destination", then you should use it
everywhere---there should be no "destination" (nor "dst") in the
code.

My point with that is you should choose the simplest name that
conveys enough information. The difficulty, as always, is
that both "simplest" and "enough information" are subjective
terms. Hopefully the "average" person will have an "average"
interpretation of simplest-with-enough-information.

Agreed. My point was just that this difficulty exists; that the
rule can be ambiguous. To which I would add that consistency in
naming is important---so the name chosen must convey enough
information in all contexts that it might appear. (If I see *d
on the left side of an assignment, the context tells me that it
is a destination. But there are obviously a lot of contexts
where "d" would not convey this information, so just "d" is not
enough information.)

 

[James Kanze]

[..]

I've seen
coding style docs with the requirement that a variable should always
have at least two letters in its name. I can't write

for (int i = 0; i < somecount; ++i)

and instead have to use 'ii':

for (int ii = 0; ii < somecount; ++ii)

Or "index".

Nobody remembered why it was established. So, common sense
and periodic reviews of the policies are important, too.

The one time I saw that, the motive given was search and
replace. Something like 's/i/j/' in vim or sed is going to wreck
havoc; something like 's/ii/jj/' will probably work.

I pointed out that all of the editing tools we used (vim, emacs,
sed, ed...) supported replacement of complete word only, e.g.
's/\<i\>/j/', and the rule was dropped.

 

[James Kanze]

Completely agreed, with one exception: Using "i" for a loop
counter is well- established and acceptable, even though it's
semantically meaningless.

Actually, for a loop _counter_, I'll use count, e.g.:

for ( int count = n ; count > 0 ; -- count ) ...

On the other hand, i, j, k... are the standard indexes in every
book on mathematics I've ever seen. If I'm iterating through
indexes, then the first should be i, the second j, etc.;
anything else is obfuscation. Of sorts: there are certainly
exceptions, e.g. two independent indexes into different things
can't both be called i.

Now if only I could find an equivalent convention for iterators.
I'm currently using iter, jter, kter, etc. Which certainly
isn't universal (and luckily I rarely have nested loops with
iterators).

 

[Arne Mertz]

grep -w i source.cpp
grep -w ii source.cpp

(if your grep doesn't support -w, use "\<i\>" and "\<ii\>" instead) And
just in case you think it's because I'm familar with grep, this is the
first time I've ever used the command.

well, I know the -w option for grep. When I was writing "grep for i"
I did not mean that /literally/, but ok, I'll provide a more
elaborated version of my statement for you:

Imgine you search the first example for "i" with some commonly used
tool such as a "search"-function in an text editor that not
necessarily supports UNIX-grep options and/or regexp-pattern
matching. That argument des not prevent me personally from using i
as a loop variable name, but it can be found as a reason in many
style guides that discourage from using single character names,
especially older style guides that were written down in times where
most editors had no regexp matching.

*gasp* Better this way?

greets
A

 

[Arne Mertz]

char *strcpy(char *source, char *dest)
{
char *ret = source;
while ((*dest = *source) != 0)
{
++dest;
++source;
}
return ret;
}

[snip]

And of course, I notice that you've adopted a convention for
abbreviating names as well: why not sourceString and
destinationString? (Objects should be qualified nouns, and
abbreviations aren't used, according to the usual naming rules.)

Well, abbreviations could be omitted, but I think source and
destination would be enough. Encoding the variable's type in the
name often is no good idea. First, you can see the types from the
function's parameter declaration. Second, in a more general case,
consider this:

std::list<std::string> optionList;

after some refactoring this is changed to

std::vector<std::string> optionList; //oops.

Encoding the type in the variable's name is duplication of the type
information (and therefore violates the DRY principle), and it can
(and eventually will) lead to misinformation if the type changes as
shown above.

greets
A

 

[James Kanze]

Arne Mertz wrote:

[...]

Encoding the variable's type in the name often is no good
idea. First, you can see the types from the function's
parameter declaration. Second, in a more general case,
consider this:
std::list<std::string> optionList;
after some refactoring this is changed to
std::vector<std::string> optionList; //oops.
Encoding the type in the variable's name is duplication of
the type information (and therefore violates the DRY
principle), and it can (and eventually will) lead to
misinformation if the type changes as shown above.

Sometimes people object that changing the type SHOULD involve
revisiting all code that uses it, but in this case the
INTERFACE only changed slightly; most code using the list will
continue to work when using a vector, so this isn't
necessarily a change that requires revisiting everything.

Do you really think so? Presumably, if the author used
std::list, instead of the more usual std::vector, it could only
be because he needed some of the special characteristics of
list, e.g. the fact that insertion and deletion don't invalidate
iterators.

 

[Arne Mertz]

But your example doesn't contain an instance of the type in
name. The object is a list of options; whether the list is
implemented using std::list or std::vector doesn't change that.

That the options would appear as a List in their graphical
representation is one thing. Still, calling it "list" in the code
would encode the graphical presentation into a variable that might
be suited in a distant module, far away from the UI-Layer of the
application. And it still would be information duplication, and if
the graphical representation changes there would be no graphical
list anymore.
But anyways, I think that if any programmer reads the word "List",
the first thing thar comes in his mind is the linked list.
I'd call that variable an optionList if and only if you have proven
that you need the performance properties of a list container, and in
addition I'd provide a comment about that and that the container
type should not be changed.
In any other case I would call the variable an "optionContainer" or
just "options".

greets
A

 

[James Kanze]

That the options would appear as a List in their graphical
representation is one thing. Still, calling it "list" in the
code would encode the graphical presentation into a variable
that might be suited in a distant module, far away from the
UI-Layer of the application. And it still would be information
duplication, and if the graphical representation changes there
would be no graphical list anymore.

Who said anything about graphical representations? It's a
"list", like in shopping list. According to the American
Heritage Dictionary, a list is "A series of names, words, or
other items written, printed, or imagined one after the other."
The most obvious implementation of a "series of [...] one after
the other" is (in my mind) std::vector, but that's beside the
point.

But anyways, I think that if any programmer reads the word
"List", the first thing thar comes in his mind is the linked
list.

I think it depends on the level you're programming at. If a
container has "list" in its name, yes. But if you're working at
the application level, in contact with users, the first thing
which comes to mind will probably be the everyday meaning.

Of course, this only applies in an English speaking context.
When I was working in Germany, the rule was to use English only
for "technical" terms (like specific data structures), and to
use German for terms from the application domain ("fachlich"),
so we'd have optionsVerzeichnis here, and list would only refer
to the data structure or the component in a GUI.

I'd call that variable an optionList if and only if you have
proven that you need the performance properties of a list
container, and in addition I'd provide a comment about that
and that the container type should not be changed. In any
other case I would call the variable an "optionContainer" or
just "options".

I'd call it optionList if that was the way my users spoke about
it. Container is, in this context, too technical---no user
would ever use the word in that sense. Options would work, but
the difference between "option" and "options" is very slight,
both in terms of entering and reading the text, and in terms of
gut feeling about the words.

 

[Noah Roberts]

But your example doesn't contain an instance of the type in
name. The object is a list of options; whether the list is
implemented using std::list or std::vector doesn't change that.

[snip]

But anyways, I think that if any programmer reads the word "List", the
first thing thar comes in his mind is the linked list.

Actually, and I'm far from a newbie, I didn't get your illustration
because I *didn't* think of "optionList" as specifying that it was a

I'd call that variable an optionList if and only if you have proven that
you need the performance properties of a list container, and in addition
I'd provide a comment about that and that the container type should not
be changed.

I do agree that embedding the type of the object in the name is usually
a really bad idea, but sometimes the type of the object and the abstract
use of the variable are the same.

In any other case I would call the variable an "optionContainer" or just
"options".

"optionContainer" could be too general. Could that include a set<>? A
queue? The use of these types would be so completely different from
list<> and vector<> as to require complete rewrite of any code using
that variable.