I inherited some code in which every class member of every class is public. I am attempting to hide the guts by writing accessors, then I can fix the problem with the type of data which is map< int, double* >.
So for the time being I have to write an accessor to a double* map.
At the moment I am getting a seg fault for writing data to the map. I havent tried accessing data stored in the map yet.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19
class mapthing
{
public:
map<int, double*> map_data;
inlinedouble& operator() (int i, int j)
{
double *arr=map_data[i];
return arr[j];
}
inlinedoubleoperator() (int i, int j) const
{
map<int, double*>::const_iterator it= this->map_data.find(i);
double *arr=it->second;
return arr[j];
}
};
Post some more code if you can ... so that we can try debugging
Edit: on the side note, you do not have to put inline infront of function if you are giving it's full definition inside the class itself. Any function which you define completely inside the class is by default inline
I don't really understand the question. What does mapthing have to do with the problems that you inherited?
As far as mapthing goes, it looks like a functor to me. Why did you overload operator()? What is this class for and why do you need to write a wrapper class for a std::map? Since map_data is public how can the class possibly be implemented with correct accessor functions? The class instance can't know if the map was changed by some other part of the program. There is no way that mapthing could know how big the array of doubles is and there is no way for it to perform any error checking to verify requests by the user since the object can be initialized and changed at any time by users of the object.
"mapthing" is a reduced version of the class I am currently working on privatizing.
The class already exists and is being used in a large working program. I am attempting to refactor the program.
All of the items kempofighter lists are things I am trying to fix. I am adding accessors so that I can replace all the instances where the object is accessed externally. Then I can move the data to private and use a normal data structure like a 2-d array or a vector of vectors.
The class does know how big it is (sorry I left that out):
1 2 3 4 5 6 7
class csignature
{
public:
int rows;
int cols;
...
Here is a simple program that has everything(probably) that could happen to a mapthing's data.:
Your second post is on the right track. Where is memory allocated for the double array? I can see why the program would segfault given what you have posted. Setting the rows,cols ints don't change the fact that the map is still empty. Then you make this call.
a(i,j)=i+j;
So that calls the non-const version of operator() which attempts to return a reference to an element of the double array which has never been created in the first place.
Since nothing was ever inserted into the map, map_data[i] causes a pair to be inserted into the map. The double* is default initialized since what is returned is a pointer to nothing. then you attempt to access the arr with j but the arr doesn't point to anything.
1 2
double *arr=map_data[i];
return arr[j];
First, you understand how operator[] of a map works correct? It will search for the key and if it doesn't find a value for the key specified it will construct the key/value pair and then return a reference to the value associated with the key. I can appreciate what you are doing but you are going to have to design a completely new interface that allows the user to specify the parameters and the object will have to construct the map and the underlying double array on its own. The interface functions used to access the data must be safe. You are going to have to validate the input params first and possibly throw exceptions if the params don't make sense, otherwise the program could simply crash or exhibit undefined behavior.
I really DO NOT want to use map< int, double* > . I hope to replace it with vector<vector<double*> > or a Fortran style matrix so I can use CLAPACK. I am trying to abstract away the data so I can change it. without affecting functionality
Here is my second try, I get a syntax error: no matching function call for insert line
1 2 3 4 5 6 7 8 9 10 11 12
double& operator() (int i, int j)
{
map<int, double*>::iterator it = map_data.lower_bound(i);
if (it == map_data.end() || map_sig_data.key_comp()(i, (*it).first)){
double *newrow = newdouble[cols];
it = map_sig_data.insert(it, newrow);
}
return (*it).second[j];
}
What is the container supposed to represent? what functionality do you want to have? A multi-dimensional array of double? What is the significance of the key? If you are able to make a simple 2d array, go for it! The map would be used, in my opinion, if the keys were non-contiguous values. If all you need is a multi-dimensional array, then the std::map is not the way to go. Would you just want a std::vector<std::vector<double> >?
The reason that it fails to compile the insert is because you are attempting to insert a pointer to an array. the value_type for this map is std::pair<int, double*>.