-
Yes the array will be deallocated.
Change the function to:
double * GetArrayFromVector( std::map<std::string, double> m, vector<double> &vec, char ** names, int count )
{
vec.clear();
vec.reserve(m.size());
for (int i=0; i<count; ++i)
{
if(!names[i]) return 0;
std::map<std::string, double>::iterator iter=m.find(name[i]);
if(iter!=m.end())
vec.push_back(iter->second);
else
return 0;
}
return &vec[0];
}
Or else use boost::shared_array (also, look at boost::scoped_array)
boost::shared_array<double> GetArrayFromVector( std::map<std::string, double> m, char ** names, int count )
{
boost::shared_array<double> vec(new double[m.size()]);
for (int i=0; i<count; ++i)
{
if(!names[i]) return boost::shared_array<double>();
std::map<std::string, double>::iterator iter=m.find(name[i]);
if(iter!=m.end())
vec[i] = iter->second;
else
return boost::shared_array<double>();
}
return vec;
}
-
You can pass it by reference or new/delete it, but as posted your vector will be destructed after the function returns.
-
Yes, the vector (and the data store it holds) will be deallocated when the function ends.
Why are you creating a vector? If you want an array, just create & fill one of those..
double * GetArrayFromVector( std::map<std::string, double> m, char * names[], int count )
{
if(!names) return 0;
double* vec = new double[m.size()];
int j = 0;
for (int i=0; i<count; ++i)
{
if(!names[i]) return 0;
std::map<std::string, double>::iterator iter=m.find(name[i]);
if(iter!=m.end())
vec[j++] =iter->second;
else
return 0;
}
return vec;
}
-
Yes -- it's deallocated as soon as your return from the function, because vec is declared on the stack. The std::vector destructor takes care of freeing the memory. Since you're returning the address of a deallocated array, you're going to start messing around with deallocated memory, which is a big no-no. At best, you'll crash immediately. At worst, you'll silently succeed with a big gaping security hole.
There are two ways to fix this: (1) return the entire vector by-value, which makes a copy of the entire vector, or (2) return the vector via a reference parameter.
Solution 1:
std::vector<double> GetArrayFromVector(...)
{
...
return vec; // make copy of entire vec, probably not a good idea
}
Solution 2:
void GetArrayFromVector(..., std::vector<double> & vec)
{
// compute result, store it in vec
}
-
Yes the vector will be deallocated when the function exits (and the 'vec' variable goes out of scope). The pointer you return will therefore be invalid.
An alternative is to allocate the array on the heap (using the 'new' operator) and return that pointer instead, but it will be the caller's responsibility to 'delete' the pointer, which is usually frown upon.
A better alternative is to return a shared_ptr to your array.
-
vec is a local variable. Its scope is limited to the GetArrayFromVector() function only. Never return the address of a local variable. Either return the array by value:
std::vector<double> GetArrayFromVector( std::map<std::string, double> m,
char ** names, int count )
or, pass a reference to the vector as an output parameter:
void GetArrayFromVector( std::map<std::string, double> m,
char ** names, int count,
std::vector<double>& vec)
or, pass an output iterator:
void GetArrayFromVector( std::map<std::string, double> m,
char ** names, int count,
std::vector<double>::iterator vecIter)
The last two will require some careful implementation of the function definition and calling though.
Additionally, if you are game for a bit more adventure try this:
// you'd need to change the value to use when an element is not
// found in the map to something that suits your needs
double pred(std::map<char*, double> haystick, char* const needle) {
std::map<char*, double>::iterator i = haystick.find(needle);
return i != haystick.end() ? i->second : 0;
}
int main(int argc, char* argv[])
{
std::map<char *, double> m;
std::vector<char *> names;
std::vector<double> dv;
m[ "Sasha" ] = 729.0;
m[ "josh" ] = 8154.0;
names.push_back("Sasha");
names.push_back("JonSkeet");
names.push_back("josh");
// transform is part of STL's <algorithm> header
// it takes a container (actually a range -- [begin(), end())
// note it is a half-open range -----------^
// as is customary for all STL algorithms, applies the function
// or functor specified as the last parameter to each element of
// the sequence and writes the result back to another container
// specified via the output iterator -- the third argument
//
// since I have not reserved enough elements for the vector dv
// i cannot blindly use it -- i need a back_inserter to coax
// transform to push_back() instead of do an insert operation
// of course, for vectors, this is costly since reallocations
// may happen, but let's leave the performance aside for a while!
//
// ok, so what about the last parameter, you ask? it has to be an
// unary_operation. well, mostly so. but what is it that we want?
// we want to take an iterator from the original char* (string)
// array and see if there's an entry in the map. if there is one
// we retrieve the associated double value and put it in dv; else,
// we set a default value of 0 -- change it to whatever pleases you
// maybe a std::numeric_limit<double> if it works for you.
//
// you can create a functor inheriting std::unary_function and pass
// it on. that's the easy way out. but what if you already have a
// comparator, a C-style find() function? will it work? yes, it will.
// but we have to wrap it using the function adaptor std::ptr_fun
// to make the compiler happy (after all it wants a unary_function, right?)
//
// this simple scheme of things works very well, save for a last little
// glitch. the comparator actually takes two parameters -- a what to search
// and a where to search. and guess what -- the where to search is always
// fixed. so that gives us a good oppertunity to fix the first parameter to
// our map<char*, double> which is exactly what std::bind1st() does.
// surprisingly, now that you've fixed one function, you no longer have a
// binary function (one taking two arguments) but an unary one -- which is
// just what you'd pass to transform. voila!
std::transform(names.begin(), names.end(), std::back_inserter(dv),
std::bind1st(std::ptr_fun(pred), m));
std::copy(dv.begin(), dv.end(),
std::ostream_iterator<double>(std::cout, "\n"));
return 0;
}
Some interesting links:
Also check out Boost. They have done some magic with bind()!
-
Since you know count up front, there's no benefit to using an stl vector. You could simply do this:
double* GetArrayFromVector(std::map<char*, double> m, char** names, int count)
{
double* result = new double[count];
for (int i = 0; i < count; ++i)
{
if(!names[i])
{
delete[] result;
return 0;
}
map<std::string, double>::iterator iter = m.find(name[i]);
if(iter != m.end())
{
result[i] = iter->second;
}
else
{
delete[] result;
return 0;
}
}
return result;
}
Note that you are passing ownership of the allocated array to the caller. Personally, I'd try to write the code in a way that observes the RAII principle; either pass in a vector to be populated, or use a managed/shared pointer, etc. (both of these options have been suggested in other answers).
-
Divide your function on two.
Make your functions make just one action:
1. fill vector from map.
2. create array from vector.
Don't forget to pass map by const reference.
Main note: caller of the GetArrayFromVector is responsible for memory deallocation.
void FillVector( const std::map<std::string, double>& m,
std::vector< double >& v,
char ** names,
int count )
{
.......
}
double* createArray( const std::vector< double >& v )
{
double* result = new double [v.size()];
memcpy( result, &v.front(), v.size() * sizeof( double ) );
return result;
}
// and finally your function
double* GetArrayFromVector( const std::map<std::string, double>& m,
char ** names,
int count )
{
std::vector< double > v;
FillVector( m, v, names, count );
return CreateArray( v );
}
-
There is a concept call move constructors that would allow you to transfer the ownership of a resource held by a (stack-based) object to a new object. I have heard that STLport has a move_source template to accomplish this
This will be coming to C++0x.
In this case, you would be returning std::vector<double> instead of double*.
-
You could use std::auto_ptr smart pointer (but passing vector reference to your function is better solution).
Example of std::auto_ptr:
std::auto_ptr< std::vector<int> > getArray(int& count){
std::auto_ptr< std::vector<int> > vec(new std::vector<int>());
vec->push_back(10);
vec->push_back(12);
vec->push_back(14);
vec->push_back(16);
count = vec->size();
return vec;
}
int main(){
int size = 0;
std::auto_ptr< std::vector<int> > autoPtrVec = getArray(size);
int* ptr = &(*autoPtrVec)[0];
std::cout << "Size: " << size << std::endl;
for(int i=0; i<size; i++){
std::cout << "[" << i << "]=" << ptr[i] << std::endl;
}
return 0;
}
-
Kinda surprised no one has mentioned vector::swap. Have the caller pass in a reference to a vector which will have its contents replaced by the function:
void GetArrayFromVector( std::vector<double>& output, ... )
{
std::vector<double> vec(m.size());
// construct vec here...
output.swap(vec);
}
BTW: "GetArrayFromVector" and you pass in a map?
