Does this function have memory problem?

X

xz

Does this function have memory problem?

void pushSomthingIntoVector(vector<T> & v) { //T is a class
v.assign(0, T(100)) //T(int a) is a constructor of class T
v.push_back(T(50))
}

void anotherFunction() {
vector<T> v;
pushSomethingIntoVector(v);
cout << v[0] << endl;
cout << v[1] << endl;
}

T(100) and T(50) are created locally within the function
pushSomthingIntoVector(vector<T> & v), does it exsit outside of the
function
? say, in the two lines of "cout" ?

Should I change the code as follows:

void pushSomthingIntoVector(vector<T> & v) { //T is a class
v.assign(0, *(new T(100)) ) //T(int a) is a constructor of class T
v.push_back(*(new T(50)))
}
 
S

Sharad

xz said:
Does this function have memory problem?

void pushSomthingIntoVector(vector<T> & v) { //T is a class
v.assign(0, T(100)) //T(int a) is a constructor of class T
v.push_back(T(50))
}

void anotherFunction() {
vector<T> v;
pushSomethingIntoVector(v);
cout << v[0] << endl;
cout << v[1] << endl;
}

T(100) and T(50) are created locally within the function
pushSomthingIntoVector(vector<T> & v), does it exsit outside of the
function
? say, in the two lines of "cout" ?

Yes, they do exist outside the function. You pass vector by reference, hence
the changes to vector are seen outside the function too. Internally copies
of the temporary objects (that you create in pushSomthingIntoVector) are
stored inside the vector. They exist even when the function returns.
Should I change the code as follows:

void pushSomthingIntoVector(vector<T> & v) { //T is a class
v.assign(0, *(new T(100)) ) //T(int a) is a constructor of class T
v.push_back(*(new T(50)))
}

No, let std::vector do the memory management for you.

Sharad
P.S. - Check the documentation of vector's assign member function.
 
A

asterisc

Does this function have memory problem?

void pushSomthingIntoVector(vector<T> & v) { //T is a class
v.assign(0, T(100)) //T(int a) is a constructor of class T
v.push_back(T(50))

}

void anotherFunction() {
vector<T> v;
pushSomethingIntoVector(v);
cout << v[0] << endl;
cout << v[1] << endl;

}

T(100) and T(50) are created locally within the function
pushSomthingIntoVector(vector<T> & v), does it exsit outside of the
function
? say, in the two lines of "cout" ?

Should I change the code as follows:

void pushSomthingIntoVector(vector<T> & v) { //T is a class
v.assign(0, *(new T(100)) ) //T(int a) is a constructor of class T
v.push_back(*(new T(50)))

}

Let std::vector to do his job (manage memory allocations/
deallocations).
One advice though,instead of:

void anotherFunction() {
vector<T> v;
pushSomethingIntoVector(v);
cout << v[0] << endl;
cout << v[1] << endl;

try to use
for( vector<T>::const_iterator it = v.begin(); it != v.end(); ++it )
cout << *it << endl;

to be sure that your previous function push something there.

Retrieving elements from a vector won't do any bounds check and won't
throw any exception.
 
J

James Kanze

Retrieving elements from a vector won't do any bounds check
and won't throw any exception.

Using the [] operator with an out of bounds value is undefined
behavior. In any modern implementation, I would expect an
assertion failure, or something similar. (It's the case with
g++ and VC++, at least.)

And of course, if you have an unforseen out of bounds condition,
you don't want an exception; you want the program to crash.
 
J

James Kanze

James said:
Using the [] operator with an out of bounds value is undefined
behavior. In any modern implementation, I would expect an
assertion failure, or something similar. (It's the case with
g++ and VC++, at least.)
Provided you are in a mode with such assertions turned on.
VC++ won't do it in release mode.

Now Ron, you know better:). The compiler does what you tell it to
do. And VC++ (like every other compiler) offers tons of
options, not just one or two modes. The code I deliver has the
assertions turned on unless there is a real performance
problems.

It is true that typically, code compiled with them on and code
compiled with them off cannot be linked together. (Worse: at
least with g++, it can be linked without problems; it just core
dumps when you run it.) Which means that one tight loop, and
you have to recompile the entire application and all libraries
with it turned off everywhere.

This also means that if I'm delivering a library, I have to
deliver it in two versions, since I can't know in advance
whether the client code will have a performance problem
elsewhere which will require turning the assertions off.
 
J

Jerry Coffin

(e-mail address removed)>, (e-mail address removed)
says...
Does this function have memory problem?

void pushSomthingIntoVector(vector<T> & v) { //T is a class
v.assign(0, T(100)) //T(int a) is a constructor of class T
v.push_back(T(50))
}

This doesn't seem to make any sense. The vector starts out empty, so you
could just as well use:

void pushSomethingIntoVector(vector<T> &v) {
v.push_back(T(100));
v.push_back(T(50));
}
void anotherFunction() {
vector<T> v;
pushSomethingIntoVector(v);
cout << v[0] << endl;
cout << v[1] << endl;
}

Here you're taking for granted that the modification succeeds. I'd do
something like:

copy(v.begin(), v.end(), ostream_iterator<T>(std::cout, "\n"));

This will only copy out the real contents of the vector, not what you
_think_ you put there.

[ ... ]
Should I change the code as follows:

void pushSomthingIntoVector(vector<T> & v) { //T is a class
v.assign(0, *(new T(100)) ) //T(int a) is a constructor of class T
v.push_back(*(new T(50)))
}

No, you should not. You're using new to allocate objects, but you're NOT
keeping a pointer to them afterwards. That means you have no way of
deleting those objects, so the memory is inevitably leaked.
 
J

Jerry Coffin

[ ... ]
try to use
for( vector<T>::const_iterator it = v.begin(); it != v.end(); ++it )
cout << *it << endl;

While I agree with using begin() and end() to find the current size of
the vector, I do NOT agree that a for-loop is the way to go here. There
are things that get more complex when you try to use standard
algorithms, but this is NOT one of them. I'd advise using std::copy
instead.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,997
Messages
2,570,239
Members
46,827
Latest member
DMUK_Beginner

Latest Threads

Top