I am experiencing unexpected performance degradation when using vectors with classes.

I have a csv_File class that reads in a csv file and stores the contents in a 2D vector. There's a member function that allows access, e.g.

csv_File file("file.csv");
        file.access(2,2);

To access the 2,2 element.

Then, I have another class csv_Array that stores multiple csv_File objects in a vector, e.g private member vector<csv_File>
There's a member function that allows access, i.e. it returns a csv_File object, for example:

csv_Array file_array(5); //store 5 csv_File objects
        file_array.grab(0).access(2,2);

In the second line, grab returns a csv_File object (in this case, the first one) and access is a member function of the csv_File object.

However, I have noticed that the call
csv_Array.grab(0).access(2,2);
is much slower than it should be (it should be just 3 vector::at calls).

ADDITIONAL DETAILS (if necessary):
The application of this code is to load a bunch of csv files into memory before passing it to a friend class that will do some calculations with the data. Schematically, I have the following:

1) csv_Array has private member vector<csv_File> storage;
2) csv_Analysis is a class that is a friend of csv_Array
3) csv_Analysis accesses vector<csv_File> storage, which is in csv_Array
4) This access is done by passing csv_Analysis a reference to storage in csv_Array (so no copy hopefully....), e.g
public:
csv_Analysis(csv_Array &csv_block);

Thus, the call given above[file_array.grab(0).access(2,2); ]
actually has one additional class "level" in between and is more like

csv_Analysis analysis_Object(file_array); 
        analysis_Object.grab(0).access(2,2);

where grab acts in the same way, and is also defined as a member function of csv_Analysis class.

Edited 5 Years Ago by nocloud: n/a

Yes, I think I should also include the relevant function prototypes, they are

//Access in csv_File
    std::string access(int row, int column);

    //grab in csv_Array and csv_Analysis
    csv_File grab(int index);

Edited 5 Years Ago by nocloud: n/a

You should be returning by reference not by value

std::string & access(int row, int column);

csv_File & grab(int index);

You should be returning by reference not by value

std::string & access(int row, int column);

csv_File & grab(int index);

And also provide a const reference version

Can you provide some details on how to make it a const reference? I tried changing the returns to references, e.g string& instead of string, but this gives a compiler error about invalid initialization of non-const reference...

Try the following:

// return a non-cost reference
    std::string & access(int row, int column);
    // returns a const reference, second const indicates that the method
    // doesn't alter the grid-instance that you're accessing from
    const std::string & access(int row, int column) const;

    ...

    csv_File & grab(int index);
    const csv_File & grab(int index) const;

If you're still getting compiler errors, provide more of the source code and specify which line is generating the error.

Thanks, this seems to mostly get the code working, however, I am still getting an error from another member function that I haven't listed here.

The call that throws the error is

vector<int> time;
current_data.grab(0).dump2ivector(1,time);

dump2ivector is declared as:

void csv_File::dump2ivector(int column, vector<int> &data) {....}

where {....} has inside of it a line like

data.push_back(atoi(access(0,0).c_str()));

Is there some problem with passing in a reference to a member function? That is the only thing I see special about dump2ivector

More specifically, the error is

error: no matching function call to csv_File::dump2ivector(int, std::vector<int. std::allocator<int> >&) const

Edited 5 Years Ago by nocloud: n/a

You need to mark the dump2ivector function as const, like so:

void csv_File::dump2ivector(int column, vector<int> &data) const {....} //notice const at the end.

You need to add const qualifiers in your function's declaration and definition.

//...

class csv_File
{
    //...

    void dump2ivector(int column, vector<int> &data) const;

    //...
};

//...

void csv_File::dump2ivector(int column, vector<int> &data) const { /*...*/ }

//...

EDIT: Too slow...

Edited 5 Years Ago by m4ster_r0shi: n/a

Thanks, this seems to mostly get the code working, however, I am still getting an error from another member function that I haven't listed here.

The call that throws the error is

vector<int> time;
current_data.grab(0).dump2ivector(1,time);

dump2ivector is declared as:

void csv_File::dump2ivector(int column, vector<int> &data) {....}

where {....} has inside of it a line like

data.push_back(atoi(access(0,0).c_str()));

Is there some problem with passing in a reference to a member function? That is the only thing I see special about dump2ivector

More specifically, the error is

error: no matching function call to csv_File::dump2ivector(int, std::vector<int. std::allocator<int> >&) const

Also don't name it time, because that might cause a name clash. Call it something like timeList or something.

Adding const did the trick. Can somebody explain why that const is required on the other member functions as well?

The const at the end of a member function means that the member function will not and cannot modify the state of the object on which the member function is called on.

A member function has a hidden parameter, the "this" pointer which points to the object on which the member function is called. In that sense, you could picture it like this:

struct Foo {
  int value;
  void Bar() {
    std::cout << this->value << std::endl;
  };
};

int main() {
  Foo f;
  f.value = 42;
  f.Bar();
};

//the above could be considered equivalent to:
struct Foo {
  int value;
};

void Foo_Bar(Foo* this) {
  std::cout << this->value << std::endl;
};

int main() {
  Foo f;
  f.value = 10;
  Foo_Bar(&f);
};

With a const at the end of the member function declaration, the above becomes:

struct Foo {
  int value;
  void Bar() const {
    std::cout << this->value << std::endl;
  };
};

int main() {
  Foo f;
  f.value = 42;
  f.Bar();
};

//the above could be considered equivalent to:
struct Foo {
  int value;
};

void Foo_Bar(const Foo* this) { //notice that "this" is now a pointer to const Foo object
  std::cout << this->value << std::endl;
};

int main() {
  Foo f;
  f.value = 10;
  Foo_Bar(&f);
};

That is the picture to keep in mind. cv-qualifiers at the end of the member function declarations are the cv-qualifiers of the object to which the 'this' pointer points to.

This was necessary in your case because once you have a const object, you can only call const member functions on it.

Edited 5 Years Ago by mike_2000_17: n/a

This question has already been answered. Start a new discussion instead.