-2

I have a vector that will have an unknown number of rows and 3 columns. The vector should be constructed as follows: a statistical test is made, if it pass a threshold the vector should store infos about it. What I am doing is:

vector< vector < int > > validated_edge_list;
validated_edge_list.resize(1);
validated_edge_list.at(1).resize(3);

for(int i = 0; i < e ; i++)
{
    p = gsl_cdf_hypergeometric_P(edge_list[i][2],
                                 k_vec[edge_list[i][1]],
                                 M-k_vec[edge_list[i][1]],
                                 N_vec[edge_list[i][0]]); // n2_matrix[i][j] = M-k_matrix[i][j]

    if (p <= bonferroni_lvl)
    {
        validated_edge_list[c][0] = edge_list[i][0];
        validated_edge_list[c][1] = edge_list[i][1];
        validated_edge_list[c][2] = edge_list[i][2];
        c = c + 1;
        validated_edge_list.resize(c+1);
        validated_edge_list.at(c+1).resize(3);
    }
}

As you can see I am manually adding a new raw each time. It gives me the following error:

terminate called after throwing an instance of 'std::out_of_range'
what():  vector::_M_range_check: __n (which is 1) >= this->size() (which is 1)
Aborted (core dumped)

I can assume that I am doing something wrong and I also think that I should use the push_back option, but I don't know how.

How can I fix this? (I am new with C++.)

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Ninja Warrior
  • 47
  • 8

2 Answers2

0

I strongly suggest to linearise your data internally; vector of vectors are expensive to work with (when the outer vector relocate its memory for instance) and it's painful to read and write code which use them.

Here is a minimal idea:

template<class T, std::size_t width>
struct Matrix
{
    Matrix(std::size_t height) { _data.reserve(width*height); }
    const T& operator()(std::size_t i, std::size_t j) const { return _data[i*width+j]; }
          T& operator()(std::size_t i, std::size_t j)       { return _data[i*width+j]; }

private:
    std::vector<T> _data;
};

Usage example on coliru

YSC
  • 38,212
  • 9
  • 96
  • 149
  • I *totally* agree with your answer, however in the case of Riccardo it seems he's just beginning with C++. I think a simpler working solution would be more suitable, even if it's not as clean or as efficient. – adentinger Feb 03 '17 at 17:01
  • @AnthonyD. Well, we can't debug their code since its incomplete. This is the best I'll give them unless they post a mcve. – YSC Feb 03 '17 at 17:04
  • Thanks for the reply! I understand that vector of vectors are expensive and that the method you propose is easier both in reading and speed but you declare the size of your matrix at the start of int main() . I don't know how many rows are going to be included in the matrix, I just now that there are going to be 3 columns – Ninja Warrior Feb 03 '17 at 17:05
  • @RiccardoMarcaccioli you can dynamically resize and add items to `Matrix::_data` ;) – YSC Feb 03 '17 at 17:06
  • @YSC Forgive me if I don't know how it can be done, can you please show me an example? – Ninja Warrior Feb 03 '17 at 17:09
0

Like you said, you should use push_back. Do not use resize. push_back is made to add an element to your vector and takes care of everything. resize, however, just increases or decreases your vector's capacity.

Possible solution: (I haven't tested it, but this should give you the general idea)

vector< vector < int > > validated_edge_list;

for(int i = 0; i < e ; i++)
{ 
    p = gsl_cdf_hypergeometric_P (edge_list[i][2],k_vec[edge_list[i][1]],M-k_vec[edge_list[i][1]],N_vec[edge_list[i][0]]); // n2_matrix[i][j] = M-k_matrix[i][j]
    if (p <= bonferroni_lvl)
    {
        vector<int> single_edge_list = vector<int>(3); // Create a vector a 3 int's
        single_edge_list[0] = edge_list[i][0] ; // Fill the vector.
        single_edge_list[1] = edge_list[i][1] ; // Fill the vector.
        single_edge_list[2] = edge_list[i][2] ; // Fill the vector.

        validated_edge_list.push_back(single_edge_list); // Add it to validated_edge_list.
        c++; // You don't really need this anymore
    }

}

Note that, since the vectors inside validated_edge_list all have a length of 3, you don't need to use a vector of vectors, you could just use a structure (or class) that you could call EdgeList. It's not necessary, however.

EDIT : You could find more efficient and better ways of doing the same thing (like YSC did below), but if you're a beginner working on a small program and you don't really mind reducing efficiency then this should be easy to program and good enough.

adentinger
  • 1,367
  • 1
  • 14
  • 30
  • Thanks!! One question: do I really need to put `vector single_edge_list = vector(3); // Create a vector a 3 int's` inside the if? Why I cannot declare it just ones and overwirite it when needed? – Ninja Warrior Feb 03 '17 at 17:29
  • @RiccardoMarcaccioli Glad I could help. Regarding your question, you can do both. However, many professional programmers will tell you that declaring inside is better. Imagine you have a really complicated function with lots of variables. If you declare *all* variables at the top and *then* use them below, it will be hard for you to remember where any why they were declared. You see a variable and ask, "What is this useful for again?", then you have to scroll all the way up, find the declaration, understand it, scroll back down and ask yourself, "What about this one?", go back up etc – adentinger Feb 03 '17 at 21:35
  • Also, if the variable is declared at the top, you have to search the *whole* function to see where that variable is used. Who knows, maybe among the 300 lines of that function you didn't see a little `i++`... Whereas, if you declare as late as possible, you're 100% sure that variable hasn't been used before before it was declared. So it takes much less time to understand and debug your code. (P.S., imagine if you were reading somebody else's code and they declared their variables all the way at the top!! You don't want your workmates to think this about *your* code, do you?) – adentinger Feb 03 '17 at 21:40
  • Didn't think about that! Thanks again for your advices ! – Ninja Warrior Feb 04 '17 at 03:02
  • @RiccardoMarcaccioli No problem! By the way, if this solved your problem, you should accept either YSC's or my answer (whichever helped you most). – adentinger Feb 04 '17 at 04:26